Re: [PATCH V2 1/2] Fix delta integer overflows

From: Jeff King <peff@peff.net>
Date: Aug 10, 2017, 20:36 UTC
Message-ID: <20170810203612.lt342yq3gnfadjlp@sigill.intra.peff.net>
In-Reply-To: <xmqqmv772nmc.fsf@gitster.mtv.corp.google.com>

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.