[Toybox] Two bugs in tail toy

[Rob Landley]

On 10/27/14 05:21, luckboy at vp.pl wrote:
> I again write mail to you because I found two bugs in tail toy and wrote
> about these bugs to you and you didn't reply me.

Sorry, a little overwhelmed with todo item backlog. (And spent the whole
weekend working on sed...)

> These bugs:
> First bug is that tail added random character to stdout end when it read
> from stdin.

It's a little non-obvious from staring at it which change fixes which
bug, and I got distracted actually testing the result because in commit
1523 (back on the 14th) I broke loopfiles subtly (the test for "are we
in read only mode" was confused by O_CLOEXEC so it was using stdout
instead of stdin, which broke simple stuff like _cat_ and I didn't
notice for a bit. My bad. I'd wondered why aboriginal stopped building
but hadn't had time to track it down this weekend due to banging on sed.)

> Second bug is segmentation fault for 'tail -c 10' for big data from stdin.

Indeed, a use after free error looks like. Good catch, thanks.

That part I understand. It's the added random character I'm not seeing,
and I don't understand what the orig_len change is doing?

> Patch that fixes these bugs:
> https://github.com/luckboy/toyroot/blob/master/patch/toybox-0.5.0-tail-n-c.patch
> 
> 
> You can check occurrence of first bug by invoke command in the toybox
> directory (toybox-0.5.0):
> 
> cat toys/posix/cp.c | ./toybox tail
> 
> Exemplary output:
> ...
> 00000140  69 6e 28 29 3b 0a 7d 0a  75 |in();.}.u|
> 
> Last character is 'u'!

Not happening for me here. I note that "cp.c" has changed several times
in the past month, but testing with "hg cat -r 1472 toys/*/cp.c" (and
1521, and 1454, and that gets us back to august) isn't showing it either.

> Also, you can occurrence of second bug by invoke commands in toybox
> directory:
> 
> dd if=/dev/urandom of=test.bin bs=1024 count=8
> hexdump test.bin | ./toybox tail -c 10
> Segmentation fault
> 
> Łukasz Szpakowski.

I checked in the use after free error. I'm not against applying the
other one but I don't currently understand what it's doing and can't
reproduce the issue.

Thanks,

Rob

 1414455737.0