wryun / es-shell

es: a shell with higher-order functions
http://wryun.github.io/es-shell/
Other
313 stars 26 forks source link

`$&read`ing a NUL byte crashes the shell #93

Open jpco opened 8 months ago

jpco commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 3 months ago

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.