<div dir='auto'><div>Hello,<br><div class="gmail_extra"><br><div class="gmail_quote">On Jul 5, 2020 4:17 AM, Rob Landley <rob@landley.net> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">On 7/5/20 1:19 AM, Ariadne Conill wrote:
<br>
> The sprintf() call, while technically valid (17 bytes fits in an 18
<br>
> byte allocation) trips Alpine fortify-headers due to checking for
<br>
> allocations that could potentially overrun.
<br>

<br>
I admit it's non-ideal code, that's why it's in pending. Why does alpine build
<br>
toybox commands out of pending? They're not guaranteed to work. Heck, they're
<br>
not guaranteed to _compile_.
<br>

<br>
The toys/pending directory is more or less our version of linux/drivers/staging
<br>
except with a lower bar to be in there. (There's a README in the directory about
<br>
this: pending being empty and getting removed is one of the prerequisites for
<br>
toybox's 1.0 release.) I have very high standards for completed toybox commands,
<br>
and will happily (try my best to) fix any issue you have with them. But just
<br>
about anything that's still in pending is there because it's a can of worms to fix.
<br>

<br>
This particular command needs a surprising amount of work for its size. Here's
<br>
the first line of non-declaration non-comment code in wget_main():
<br>

<br>
  if (!(toys.optflags & FLAG_O)) help_exit("no filename");
<br>

<br>
1) the modern test is if (!FLAG(O))
<br>

<br>
2) The optargs string has | to indicate an option is required and lib/args.c
<br>
will error for you if it isn't supplied, so this line can go away and be
<br>
replaced with literally a single character in the command description.
<br>

<br>
3) requiring -O is silly, it should be able to get it from the URL and/or the
<br>
redirect. The design is wrong and missing a chunk of plumbing to do that.
<br>

<br>
So that's three things wrong with the first line of code, from trivial "there's
<br>
a convenience macro to make this more readable" to "submitter didn't know how to
<br>
use the available infrastructure" to "what it's trying to accomplish is wrong".
<br>

<br>
When I do find a large block of time to sit down and focus on this, I'd probably
<br>
want to write httpd at the same time because I need to come up to speed on what
<br>
all the funky "headers:" the server and client exchange are/mean (I started
<br>
doing that once but got pulled away) and that domain expertise is the same at
<br>
sending and receiving ends so might as well implement both while it's fresh in
<br>
my mind. (And then I need to come up with tests for interrupting and resuming
<br>
partial downloads and so on.)
<br>

<br>
And some of the headers are for things like sending gzipped data through these
<br>
connections which I have deflate infrastructure for in toybox already (but I
<br>
need to circle back around to writing the gzip compression side, which is
<br>
currently a stub because I hit the issue "if I want to be binary compatible with
<br>
gzip output, when do you perform dictionary flushes and when do you use the
<br>
builtin dictionary instead of saving one?" which would be a lot easier to answer
<br>
if I let myself read the other implementation's source code but I try to avoid
<br>
doing for licensing reasons even when it ISN'T gnu code I try to avoid reading
<br>
for headache/nausea/rage reasons; I should break down and read the old busybox
<br>
1.2.1 code for this since I've already _read_ that as the project's maintainer
<br>
and can't really be _more_ contaminated there now can I).
<br>

<br>
There's a bunch of other issues, what do I have bookmarked in my todo list...
<br>

<br>
http://lists.landley.net/pipermail/toybox-landley.net/2016-February/008006.html
<br>

<br>
Yes, cleanup for this command has a dependency graph.
<br>

<br>
At the moment, I'm trying hard to finish the new shell implementation before
<br>
opening another can of worms. It's big and complicated and I put it off a LONG
<br>
time because I knew it would eat my life for a while. I believe I'm more than
<br>
halfway through it, but how _much_ more I couldn't tell you...
<br>

<br>
> The call is pointless anyway -- as we are appending a constant to
<br>
> another constant, it is better to just let the compiler do so and
<br>
> calculate the size.  This is supported by ISO C89 and later, and
<br>
> thus any compiler that would be used to compile toybox.
<br>

<br>
I applied the patch, but given the above context, the question "are you actually
<br>
using these commands on alpine"...?
<br></p></blockquote></div></div></div><div dir="auto">We are not yet using toybox.</div><div dir="auto"><br></div><div dir="auto">It is available in the testing repo, as defconfig.  This work is about evaluating toybox as a possible eventual replacement to BusyBox down the line, but a lot of things will need to happen before we get there I think.  I am not opposed to doing those things as I believe toybox is probably more sustainable in the long term due to not being a licensing mess.</div><div dir="auto"><br></div><div dir="auto">For example, several of the applets contain code that, bluntly, I would expect either from the demoscene or in an IOCCC entry.  While there is something to be said about squeezing the maximum possible binary size, I think in 2020 transparency in implementation is more important, so that we and others can effectively audit the code.</div><div dir="auto"><br></div><div dir="auto">If you agree, I would be happy to refactor some of that stuff.</div><div dir="auto"><br></div><div dir="auto">In this case, I wanted to evaluate the wget command verses the BusyBox one.  It crashed so I sent you a patch fixing it :)</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">
I mean, good luck if you are. Android's using quite a few itself:
<br>

<br>
$ grep pending aosp/external/toybox/Android.bp
<br>
    # Edit the relevant `srcs` below, depending on where the toy should be
<br>
    "toys/pending/dd.c",
<br>
    "toys/pending/diff.c",
<br>
    "toys/pending/expr.c",
<br>
    "toys/pending/getopt.c",
<br>
    "toys/pending/tr.c",
<br>
    "toys/pending/getfattr.c",
<br>
    "toys/pending/lsof.c",
<br>
    "toys/pending/modprobe.c",
<br>
    "toys/pending/more.c",
<br>
    "toys/pending/readelf.c",
<br>
    "toys/pending/stty.c",
<br>
    "toys/pending/traceroute.c",
<br>
    "toys/pending/vi.c",
<br>

<br>
But the ones android's using are my priority to clean up (and most of them are
<br>
in better shape with a lot less work required, more auditing than reconstructions).
<br></p></blockquote></div></div></div><div dir="auto">readelf is quite problematic actually.  There are numerous warnings generated by static analysis of that applet, for example a specially crafted ELF file with a 2^33 section size will likely crash your parser.</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">
There's a reason the build issues a red line warning for using stuff out of the
<br>
pending directory, and "make defconfig" does not include anything from pending.
<br>

<br>
Rob
<br>

<br>
P.S. You'll have to imagine the "gomenasai" and kowtow for being perpetually
<br>
slow at getting everything done. I am aware I suck, while having very high
<br>
standards. Ira Glass implied one grows out of this
<br>
(https://www.youtube.com/watch?v=91FQKciKfHI) but so far...
<br>
</p>
</blockquote></div>Ariadne</div></div></div>