Re: [PATCH v2] elf: Check for empty tokens before dynamic string token expansion [BZ #22625]

[Aurelien Jarno <aurelien at aurel32 dot net>]

On 2017-12-19 14:16, Florian Weimer wrote:
> On 12/18/2017 09:42 PM, Aurelien Jarno wrote:
> > -  return _dl_dst_substitute (l, s, result, is_path);
> > +  char *retval = _dl_dst_substitute (l, s, result, is_path);
> > +
> > +  /* If substitution of dynamic string tokens resulted to an empty string,
> > +     return NULL as in case of insufficient memory.  */
> > +  if (__glibc_unlikely (*retval == '\0'))
> > +    {
> > +      free (result);
> > +      return NULL;
> > +    }
> > +
> > +  return retval;
> 
> I'm not really happy with this.  OOM and a zero-string expansion are very
> different things.  We seem to have other abuses in the existing code, but I
> don't think we should add further instances.

That's something changed from the first version. I can rollback that
change, and add back the check or empty string in fillin_rpath.

> Furthermore, I'm not sure if the fix is robust enough.  There is another bug
> in the $ORIGIN DST component skipping:  $ORIGIN eats the trailing colon.
> This is because this code:
> 
>       else
> 	{
> 	  *wp++ = *name++;
> 	  if (is_path && *name == ':')
> 
> happens when name is at the ':' after $ORIGIN processing, so only the next
> ':' after that (or the end of the string) terminates this component, and the
> call to the is_trusted_path_normalize covers two components mushed together
> instead of one.  So the check likely fails, and both path elements are
> skipped.  In some cases, this results in an empty string where it would not
> usually be non-empty.

While this is indeed a problem in _dl_dst_substitute, it's not an issue
in this case as expand_dynamic_string_token and thus _dl_dst_substitute
do not get called from fillin_rpath with a colon. Indeed since commit
b75891075bec (which introduced this bug), expand_dynamic_string_token
is called after the split of the path by colons instead of before.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Attachment: signature.asc
Description: PGP signature