Re: [PATCH] gst-tool: Fix ASAN issue on comparing options

On 24/11/2017 10:30, Holger Freyther wrote:
> 
>> On 12. Feb 2017, at 13:16, Holger Freyther <[email protected]> wrote:
>>
> 
> Dear Paolo,
> 
> 
>>> Wouldn't the '\0' mismatch first?
>>
>> "Both strings are assumed to be n bytes long". I guess an optimized memcmp will fetch 32/64 bytes at a time (manual loop unrolling?).
> 
> 
> I noticed I didn't push this change yet. I think in the case ASAN
> reported, name and all_opts->name are of different size and 1-3 bytes
> after the \0 will be fetched? If I see this correctly right now we want
> to check if name is a prefix of all_opts->name? For this to be true
> all_opts->name must be at least as long as the prefix (name)? Am I wrong?

Lee is right, see also https://trust-in-soft.com/memcmp-requires-pointers-to-fully-valid-buffers/

However, I am not sure strlen is needed.  The code is:


  if (!p)
    len = strlen (name);
  else
    len = p++ - name;

  for (all_opts = long_opts; all_opts; all_opts = all_opts->next)
-   if (!memcmp (name, all_opts->name, len))
+   if (strlen(all_opts->name) >= len && !strncmp (name, all_opts->name, len)) 

so len <= strlen(name): there is no NULL byte in the first LEN
bytes of NAME.  For the first LEN bytes to be equal in the two
arguments to strncmp, there must be no NULL byte in all_opts->name,
and thus strlen(all_opts->name) >= len too.

Thanks,

Paolo