Thread (1 message) 1 message, 1 author, 2017-08-11

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

From: Junio C Hamano <hidden>
Date: 2017-08-11 18:43:34

Jeff King [off-list ref] writes:
On Thu, Aug 10, 2017 at 01:07:07PM -0700, Junio C Hamano wrote:
quoted
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.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help