Re: [PATCH] Improve DST handling (Bug 23102, Bug 21942, Bug 18018, Bug, 23259, CVE-2011-0536 ).

[Florian Weimer <fweimer at redhat dot com>]

On 06/06/2018 07:02 AM, Carlos O'Donell wrote:
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 431236920f..13263212d5 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -177,63 +177,89 @@ is_trusted_path_normalize (const char *path, size_t len)
>     return false;
>   }
>   
> +/* Given a substring starting at NAME, just after the DST '$' start
> +   token, determine if NAME contains dynamic string token STR,
> +   following the ELF gABI rules for dynamic string tokens:
>   
> +   * Longest possible sequence using the rules (greedy).
> +
> +   * Must start with a $ (enforced by caller).
> +
> +   * Must follow $ with one underscore or ASCII [A-Za-z] (enforced by
> +     caller via STR comparison) or '{' (start curly quoted name).

“enforced by caller via STR comparison”: I don't see this in the 
remaining code.  So $ORIGIN and $PRIGIN are currently treated the same?

> +   * Must follow first two characters with zero or more [A-Za-z0-9_]
> +     (enforced by caller) or '}' (end curly quoted name).
> +
> +   If the sequence is a dynamic string token matching STR then
> +   the length of the DST is returned, otherwise 0.  */
>   static size_t
> -is_dst (const char *start, const char *name, const char *str, int secure)
> +is_dst (const char *name, const char *str)

I'm not entirely happy about the choice of parameter names. 
name/reference or input/name would work better IMHO.

>         /* Point again at the beginning of the name.  */
>         --name;

This assignment is now dead, and the comment is wrong.

(Still need to look at the rest of the patch.)

Thanks,
Florian