Re: [PATCH] parse_tag_buffer(): do not prefixcmp() out of range

2011/2/16 Junio C Hamano [off-list ref]:

quoted

-     if (prefixcmp(bufptr, "tag "))
+     if (bufptr + 4 < tail && !prefixcmp(bufptr, "tag "))
+             ;               /* good */
+     else
              return -1;
      bufptr += 4;
      nl = memchr(bufptr, '\n', tail - bufptr);

If there weren't enough bytes between bufptr and tail, prefixcmp may still
match with "tag " while later part of the matched string might be coming
from trailing garbage outside our memory.  Unless we correctly fail the
prefixcmp() part, memchr() would be fed negative value, no?

Yes, memchr() would be fed negative, but prefixcmp() already steps
outside allocated memory. I believe that caused valgrind error Thomas
reported (although I couldn't reproduce it).

Nguyễn Thái Ngọc Duy [off-list ref] writes:

quoted

There is a check (size < 64) at the beginning of the function, but
that only covers object+type lines.

Strictly speaking the current code is still correct even if it
accesses outside 'data' because 'tail' is used right after
prefixcmp() calls.

What do you mean by this?  I don't get it.

Because memchr() would be fed negative, memchr() would fail so the
code is still correct.
-- 
Duy