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

[Carlos O'Donell <carlos at redhat dot com>]

On 06/08/2018 01:21 AM, Florian Weimer wrote:
> On 06/08/2018 06:14 AM, Carlos O'Donell wrote:
>> The only time the code you quote is executed, this code:
>>
>>   338           else if (len != 0)
>>   339             {
>>   340               /* We cannot use this path element, the value of the
>>   341                  replacement is unknown.  */
>>   342               check_for_trusted = false;
>>   343               wp = last_elem;
>>   344               break;
>>   345             }
>>
>> Is when we find a DST we know, say $LIB, but DL_DST_LIB is invalid
>> e.g. set to -1, indicating that $LIB's value is unknown, in which case
>> [$ORIGIN/../$LIB] is entirely considered unknown, and*discarded*  (which
>> is what 'wp = last_elem' does).
>>
>> For v4 I'm going to clean up _dl_dst_substitute to point out that we
>> only take individual path elements of a multi-path sequence.
>>
>> I believe this answers your question. Please clarify if I have not.
> 
> Yes, it does.  What the quoted code actually does is something like this, right?
> 
>   /* Return an empty string to tell the caller to drop the element.  */
>   *result = '\0';
>   return;

Exactly, and you'll see I do just that in v4 patch to make things clearer.

            {
-             /* We cannot use this path element, the value of the
-                replacement is unknown.  */
-             wp = last_elem;
-             break;
+             /* We found a valid DST that we know about, but we could
+                not find a replacement value for it, therefore we
+                cannot use this path element and discard it.  */
+             *begin = '\0';
+             return result;
            }

Cheers,
Carlos.