[Toybox] [PATCH 2/2] Detect all errors with xreadall/xwrite.

[Rob Landley]

On 8/15/21 7:12 AM, Samanta Navarro wrote:
> On Sun, Aug 15, 2021 at 06:57:54AM -0500, Rob Landley wrote:
>> > It is too difficult for me to figure out the direction in which the
>> > toybox design moves and where it comes from and which kind of errors are
>> > accepted or even embraced (like data types, signed overflows).
>> 
>> I disagree with you on a design decision, therefore I am inscrutable?
> 
> I did not call you inscrutable. Please do not put words into my text
> which are not there. But to clarify my problems with understanding this
> project:
> 
> On one hand you think about supporting arbitrarily long lines (your
> readline discussion) on the other hand you want to limit the size of
> tar headers to PATH_MAX

No, I'm saying that historically the size of the tar headers was (past tense)
practically limited to PATH_MAX because the filesystem data they represented
was, so this wasn't a problem that needed solving before in the tar design of
historical implementations except in terms of defending from crafted attack
tarballs.

Now it potentially is, so needs fresh review. Which goes on the TODO list.

> (it is not necessarily enough because the pax
> extended headers can have multiple entries and only one of them could
> be the path) and ignore memory leaks of arbitrarily large tar streams.

I am aware of the pax headers being potentially arbitrary length, hence the "2
of 3" comment. But I don't want to SUPPORT arbitrary length (requiring arbitrary
runtime memory), and am wondering what a good "limit as defense from crafted
attack tarball" is. This requires research I haven't had the energy to do yet.

Speaking of "pax length limits":

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html

  The linkname field, described below, shall not use the prefix to produce a
  pathname. As such, a linkname is limited to 100 characters. If the name does
  not fit in the space provided, pax shall notify the user of the error, and
  shall not attempt to store the link on the medium.

  The maximum block size of 32256 bytes (215-512 bytes) is the largest multiple
  of 512 bytes that fits into a signed 16-bit tape controller transfer register.

Bravo. (I am TEMPTED to make the limit be 32256 bytes because then at least it
wouldn't be arbitrary: I could at least say where I go the number FROM. :)

But I'm not 100% certain that pax header support is necessary: the tar
submission I got was doing it and I preserved that, but pax also says that
lengths greater than the octal field can store should add a "size=" entry in the
pax header
(https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_03)
and we don't support that and it hasn't caused a problem? (Instead we use the
high bit set binary format.)

So I am tempted to REMOVE the 'x' support in the process of fixing this, except
for the part where proving a negative is very difficult. (We don't ever generate
it and I don't know of a modern tool that does? We have SOME support for
historical pre-Y2K tarballs, but not all of them? I don't have good test cases
here...)

Mostly I've been deferring this until adding selinux support so I can see what
size limit THAT needs. (Or make puppy eyes at Elliott who has way more
experience there than I do. But that's kind of wrapped into the whole "adding
selinux support to cpio and initramfs" hairball that crops up every year or so
on linux-kernel...)

> On one hand you disagree on portability or implied documentation of
> data types on the other hand you consider PATH_MAX although it is a
> portability macro itself.

"Portability" to what? #include <linux/limits.h> still #defines it to 4096
today, and toybox targets Linux. (Meanwhile posix helpfully says we'd be
compliant supporting 256. No really:
https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/limits.h.html
.)

I am aware the VFS limit was removed*, I watched it happen and blogged about it
at the time:

  https://landley.net/notes-2008.html#05-01-2008
  https://landley.net/notes-2009.html#02-11-2009

But that doesn't mean symlinks are now infinite length, their size limit is now
per-filesystem:

https://stackoverflow.com/questions/36757577/in-linux-can-the-value-of-a-symlink-be-longer-than-path-max/36759498

As you can easily test: "ln -s $(seq 1 10000 | xargs | sed s/./x/g) boom" tries
to create a ~48k symlink and the result is "file name too long". And no it's not
the VFS limit of 255 bytes for a single filename:

  $ ln -s $(seq 1 10000 | xargs | sed 's@ @/@g;s@[^/]@x at g') boom

Same error.

Rob

* Not that PATH_MAX limits have even been entirely cleaned out of the VFS even
today:

  https://github.com/torvalds/linux/blob/v5.14-rc6/fs/namei.c#L156