[Toybox] ASAN errors in toysh

[Eric Roshan-Eisner]

Some new ones:
$ toybox sh -c '['
$ toybox sh -c '${\'
$ toybox sh -c '(0;0)'
$ toybox sh -c '$(($(<`0\'
$ toybox sh -c 'for i in;do 0;done&'
$ toybox sh -c 'f(){ f;};f <f'
$ toybox sh -c '{{,},,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,'
$ toybox sh -c $'\n$\'\375\375\375\375\''

I'll keep running the fuzzer over the weekend.

On Mon, Jan 9, 2023 at 8:39 AM Eric Roshan-Eisner <eric.d.eisner at gmail.com>
wrote:

> 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 <rob at landley.net> 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...
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20240524/1b75c15c/attachment.htm>