[PATCH v7 4/4] Add system-wide tunables: Filters

[DJ Delorie]

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>> +	    /* prog_name and the cached string are both NUL terminated.  */
>> +	    if (prog_name)
>> +	      {
>> +		if (((const char *)(td + tec->flag_offset))[0] == '/')
>> +		  {
>> +		    if (memcmp (prog_name, td + tec->flag_offset, prog_name_len) != 0)
>
> I think it should use the filter prefix here, instead fo program one: /bin/l matches
> filter [proc:/bin/ls]:
>
>   memcmp ("/bin/l", "/bin/ls", strlen ("/bin/l")
>
> Also, if the program name is longer than the filter it might resulting in OOB read:
> /bin/lsa filter [proc:/bin/ls]:
>
>   memcmp ("/bin/lsa", "/bin/ls", strlen ("/bin/lsa")

I switched to strcmp instead, which (I HOPE) handles these cases
better.

>> +	  /* See if the parsed type matches the desired type.  */
>>  	  if (tunable_list[tid].type.type_code == TUNABLE_TYPE_STRING)
>>  	    {
>>  	      /* This is a memory leak but there's no easy way around
>
> I think this would apply system-wide tunables AT_SECURE binaries by default, whereas
> GLIBC_TUNABLES is dropped for same case. There is the '$' filter, but the whole
> reason env tunables are stripped under AT_SECURE is defense-in-depth (as for other
> environment variable that might alter program execution).
>
> Due the possible wide attack surface of AT_SECURE binaries, I think it would be
> better to the inverse: system tunables should be ignored for AT_SECURE unless a 
> line explicitly opts in.

I changed the default to be "non-secure only" and added a third option
to the syntax so you can specify secure, nonsecure, or both to get
tunables.

>> +  if (filter_string != NULL)
>> +    {
>> +      clear_filter ();
>> +    }
>> +
>> +  if (memcmp ("proc", line + 1, colon - line - 1) == 0)
>
> I don't think we can use memcmp here:
>
>   * if the line has '[procXXXX:foo]' then 'colon - line - 1' will be 8 and
>     it will trigger an OOB read.
>
>   * if the linehas [pr:foo], [p:foo], even [:foo], the length will be 2/1/0
>     and any prefix of "proc" (or empty) is accepted.

I test the lengths for equality first, and use memcmp with a constant
length after, now.  Should be safe.