Thanks for the quick response. Feel free to ignore these reports until after you cut the release ;).
With the new code it could branch out and find some new ones: $ toybox sh -c 'for;i 0' $ toybox sh -c '`<`' $ toybox sh -c '"$'"'"'"' # mostly shell escape; it's just " $ ' " without spaces On Sun, Jan 8, 2023 at 12:04 PM Rob Landley <[email protected]> wrote: > On 1/8/23 01:41, Eric Roshan-Eisner wrote: > > I ran the shell through the afl++ fuzzer, and it split out a few > different ASAN > > failures for simple inputs: > > Yeah, I'm grinding through the existing ones the test suite triggers right > now, > but since it's in pending I'm trying not to (further) hold up the 0.8.9 > release > for it. > > > heap-buffer-overflow: > > $ toybox sh -c '$' > > Fixed, added test. > > > $ toybox sh -c '+()' > > Yeah, it does the same for +(.) too. The wildcard_path() plumbing's still > unfinished. The basics are sketched out but I need to cycle back to it. > (There's > a reason it's still in pending...) > > Sigh, is there any way to get ASAN to stop showing the useless "shadow > bytes" > nonsense? (And "shadow byte legend" which is meta-nonsense?) I want the > stack > traces, not your internal metadata showing how proud you are of your > implementation details. Grrr. Also, if the _first_ stack trace showing > where the > access _happened_ was actually a STACK TRACE rather than just a single > file+line > address, that would be nice. You give me a trace of where it was allocated > from > but not where the access you're objecting to occurred? Funky priorities, > that... > > Hmmm, the problem doesn't look like it's in wildcard_path(), it seems like > collect_wildcards() is filling out the deck[] wrong. (Think of > collect_wildcards() as a bit like regcomp() and wildcard_path() as slightly > regexec, but not really. When the eleventh doctor says "think of a banana" > and > then goes that's a rubbish analogy, forget the banana... yeah.) > > Sigh, adding a test for this sucks because bash defaults to extglob being > off > for -c (where in the hideous nest of rc files it parses on bringup does > that get > switched on, or is this one of those "defaults differ for different > contexts" > things bash loves to do...) And the extra-creaky bit is: > > $ bash -c 'shopt -s extglob; echo +()' > bash: -c: line 0: syntax error near unexpected token `(' > bash: -c: line 0: `shopt -s extglob; echo +()' > $ bash -c $'shopt -s extglob\necho +()' > +() > > Because the entire line is parsed before any of the commands in it are run! > Which is #*%(#&%& _NOT_ the case with "for i in a b c; do echo $i; done" > but... > ARGH. Not emailing chet. Not emailing chet. > > Ahem. Not holding up the release for more work on pending either, lemme > get back > to this... > > (And yes, I should probably redo the temporary variable names on the > second pass > of wildcard plumbing. I have a tendency to put random word association > names > into things during development, and then people complain and I remove the > references later. Calling a group wildcards a "deck" is straining it > enough, but > when I've got a second one calling it "ant" is probably unintelligible > outside > the UK. Nope, never actually been there...) > > > $ toybox sh -c '<<0;0' > > I accept that it parsed wrong, but what does... ok, that's start a HERE > document > terminated by the line "0" and then queue up running the command '0' > afterwards, > and then it runs out of input and complains about an unterminated HERE > document > without having proceeded to run the queued 0. > > $ <<0;potato > > 0 > bash: potato: command not found > > Right, test for that is just: > shxpect 'queued work after HERE' I$'<<0;echo hello\n' E"> " I$'0\n' > O$'hello\n' > > And yes bash passes it for "TEST_HOST=1 make test_sh". > > Ahem, test added, lemme come back to actually _fixing_ this. Trying to cut > a > release... > > > $ toybox sh -c '{$,}' > > That's actually the same test as trailing $ above, already fixed. (I can > add a > specific test for it, but the actual bug was trailing $ because {$,} > expands to > one argument that's just "$" and then the trailing , drops out because "" > is > ignored in certain circumstances. Interesting corner case actually, you can > bring it BACK by appending it to an empty string, ala: > > $ bash -c 'for i in ""{$,}; do echo ="$i"=; done' > =$= > == > $ ./sh -c 'for i in ""{$,}; do echo ="$i"=; done' > =$= > == > > And yes, mine is getting that right. And I _think_ I already have a test > for it. > Eh, add another anyway just to be sure... > > > floating-point-exception: > > $ toybox sh -c '((0%0))' > > (I actually did this one first.) > > And /= 0. Added two checks and four tests to sh.test. (Catching and > handling the > signal without leaks is unnecessarily awkward. I'd need a siglongjmp() > stack > mechanism, and would prefer the plumbing just not _cause_ those...) > > > Also found some ASAN failures on the vi command. > > > > heap-buffer-overflow: > > $ echo p > input; toybox vi -s input ascii.txt > > stack-buffer-overflow: > > $ echo s000000000000000 > input; toybox vi -s input ascii.txt > > Understood, but I haven't even _started_ review/cleanup of that one yet. > > Moving targets tend to get bumped down the todo list, and even if I _was_ > focusing on something but it moved out from under me I may not get back to > it > for _years_. Happend more than once on dd.c, and I should cycle back to > man.c. > Both are low hanging fruit at this point. I'm waiting to see if anything > in the > entire Linux From Scratch build under than the kernel (which I have a > patch for) > actually _uses_ bc, since anybody born after 1980 is either going to use > $((math) or just python -c 'print 37/4.2' . Not that I want to _encourage_ > that. > > > -Eric > > Rob > > P.S. Remind me to let your original email through moderation when I get > home... >
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
