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

[Florian Weimer <fweimer at redhat dot com>]

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.

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.

Thanks,
Florian