[Toybox] [PATCH] tar: Fix support for long symbolic links

[Daniel Mentz]

On Wed, Mar 20, 2019 at 7:24 PM Rob Landley <rob at landley.net> wrote:
> On 3/19/19 6:38 PM, Daniel Mentz via Toybox wrote:
> > -// TODO: does this need NUL terminator?
> >      if (strlen(lnk) > sizeof(hdr.link))
>
> The todo item you erased there was a note to self about checking what happens
> when a value fits exactly in the field and thus there is no NUL terminator, and
> whether the read side was handling that correctly. (Similarly, in theory a

If I'm not mistaken, the code on the read side that handles the
tar.link field is the following:

if (!TT.hdr.link_target && *tar.link)
  TT.hdr.link_target = xstrndup(tar.link, sizeof(tar.link));

The manpage of strndup reads as follows:

"char *strndup(const char *s, size_t n);
If s is longer than n, only n bytes are copied, and a terminating null
byte ('\0') is added."

Hence, if tar.link does not contain a terminating null byte, strndup()
will add one. So, no, it does not need a NULL terminator.

>
> >        write_longname(lnk, 'K'); //write longname LINK
> > -// TODO: this will error_exit() if too long, not truncate.
> > -    xstrncpy(hdr.link, lnk, sizeof(hdr.link));
> > +    strncpy(hdr.link, lnk, sizeof(hdr.link));
>
> That todo was because it should be strlcpy(), but unfortunately Ulrich Drepper
> was an asshole and the committee that replaced him is very, very slow. As in
> this was 2014: https://lwn.net/Articles/612244/

I believe that strncpy is the correct function, since the string does
not need to be NULL terminated if it's 100 bytes long (i.e.
sizeof(hdr.link)). If you were to use strlcpy(), you would actually
lose one useful byte i.e. you wouldn't be able to fit a string that is
exactly 100 bytes long (you'd truncate it to 99 bytes and add a NULL
byte that's not required).
I briefly looked at a tar file generated by GNU tar, and hdr.link
wasn't NULL terminated there either (I had GNU tar store a symbolic
link where the target was longer than 100 bytes).