[Toybox] [PATCH] tar: fix heap buffer overrun.

[Rob Landley]

On 10/13/20 4:19 PM, enh via Toybox wrote:
> tar was assuming the old behavior of dirtree_path() where there was
> always a spare byte free at the end.

It's not the old behavior, tar.c is doing:

  i = 1;
  name = hname = dirtree_path(node, &i);
...
  // Consume the 1 extra byte alocated in dirtree_path()
  if (S_ISDIR(st->st_mode) && name[i-1] != '/') strcat(name, "/");

The "1 extra byte" is because we fed i = 1 into &i for dirtree_path(), which
should result directly in extra space at the end because that function does:

  ll = len = plen ? *plen : 0;
  for (nn = node; nn; nn = nn->parent)
    if ((ii = strlen(nn->name))) len += ii+1-(nn->name[ii-1]=='/');
  if (plen) *plen = len;
  path = xmalloc(len)+len-ll;

Starting len is value fed in, length of each non-empty node is added in minus
trailing slashes we'd remove, and then it's saved back out and we allocate the
right amount but back up the initial *plen amount from the end (saved in ll but
honestly I could just transpose those last two lines and not need the temp) so
the new "fill it in backwards" logic winds up at the start of the string and
it's just empty space.

> Since removing that seems to have
> been an intentional change to dirtree_path(), change the caller to
> resize the string itself.

I'd rather get the allocation right in the first place (do we need i = 2, if so
why?), but I leave for the airport to fly back to Japan in 2 hours. (Part of the
reason I've been so distracted lately, it's not JUST focusing on sh.c. :)

> Caught by ASan.

Operating on what path?

Rob