Jeff King <p...@peff.net> writes:

> On Thu, Aug 10, 2017 at 01:07:07PM -0700, Junio C Hamano wrote:
>
>> Perhaps we should teach the receiving end to notice that the varint
>> data it reads encodes a size that is too large for it to grok and
>> die.  With that, we can safely move forward with whatever size_t
>> each platform uses.
>
> Yes, this is very important even for "unsigned long". I'd worry that
> malicious input could cause us to wrap to 0, and we'd potentially write
> into a too-small buffer[1].
>
> There's some prior art with checking this against bitsizeof() in
> unpack_object_header_buffer() but get_delta_hdr_size() does not seem to
> have a check.
>
> -Peff
>
> [1] In most cases it's _probably_ not a vulnerability to wrap here,
>     because we'd just read less data than we ought to. But it makes me
>     nervous nonetheless.

As I said in my other message in the thread, as long as the callers
of get_delta_hdr_size() are written correctly, it should be OK.  And
patch_delta() should be OK, even for "unsigned long" when it is too
small.  It just will not produce correct result and instead abort,
and the patch under discussion fixes that.


Reply via email to