Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$&reading a NUL byte crashes the shell #93

Open
jpco opened this issue Feb 26, 2024 · 3 comments
Open

$&reading a NUL byte crashes the shell #93

jpco opened this issue Feb 26, 2024 · 3 comments
Labels

Comments

@jpco
Copy link
Collaborator

jpco commented Feb 26, 2024

The easiest way to repro is using trip.es, which already has a NUL byte in it for testing this kind of thing.

; { while {!~ <=$&read ()} {} } < trip.es
gc.c:550: assertion failed (strlen(ns) == n)
IOT instruction--core dumped

The problem here is pretty obvious based on the error message: strlen(s) is finding a NUL byte after N bytes, but $&read has actually gcallocd more than N bytes, so this assertion fails. In general, I think the shell has a rule: strings must not contain NUL bytes, except as the terminator.

The simplest way to stop the crashing would probably be to do the same thing the shell does when reading shell input in get() -- just skip over any NUL characters and warn. There could be a smarter thing to do, though. Maybe split on NUL so that $&read returns any lines containing NUL as a multiple-element list? That could compose well with the GNU xargs -0 pattern, but now it feels like we're getting back into the mire of trying to bikeshed fancy behaviors for $&read -- my vote is probably to keep it simple and just skip'n'warn.

(Oh, while I'm here, inputting echo \0 to the shell causes weird behavior: it prints "bad backslash escape" every other time, and can cause the next command to fail:

; echo \0
; ps
bad backslash escape

So that's odd.)

@jpco
Copy link
Collaborator Author

jpco commented Feb 26, 2024

Oh that's funny. The fix for this echo \0 buglet is exactly the /* TODO: check previous character? rc's last hack? */ TODO in scanerror().

@jpco
Copy link
Collaborator Author

jpco commented Mar 22, 2024

Actually, maybe the simplest fix for now is to simply throw an error when $&read encounters a NUL. Then a crash could be turned into an exception, which is strictly an improvement, and the exception could still later be turned into a more reasonable handling.

@memreflect
Copy link
Contributor

The issue is caused by gcndup() expecting a null-terminated string of length n or greater, yet sealcountedbuffer() invoked by $&read instead provides the number of bytes stored in the Buffer, which is only a problem when the Buffer contains a NUL.

To remedy this, $&read should probably consider NUL as a newline to match the behavior of %backquote \n:

; echo <={%count `` \n {find /bin -print0}}
53
; find /bin -print0 | wc -l
       0

I don't like the magic behavior, but it seems like a better option than overhauling the entire codebase to internally use counted buffers at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants