Nicolas Pitre <nico@xxxxxxxxxxx> writes:
> I didn't spend the time needed to think about this issue and your
> proposed fix yet. However I think that using sizeof(delta_head)-1
> makes the code a bit confusing. At this point i'd use:
>
> int size = sizeof(delta_head) - 1;
>
> and use that variable instead just like it is done in
> unpack_compressed_entry() to have the same code pattern.
Sounds good. Here is a reroll with a bit more explanation.
-- >8 --
From: Junio C Hamano <gitster@xxxxxxxxx>
Date: Tue, 20 Oct 2009 12:40:02 -0700
Subject: [PATCH] Fix "corrupt input stream" check while reading from packfiles
An ealier "fix" made us break out of the loop when we get Z_BUF_ERROR back
from inflate(), and either the input stream still had some data to
consume, or we have already got the full output we expected.
This is the same kind of mistake as we corrected with 456cdf6 (Fix loose
object uncompression check., 2007-03-19); it is valid for inflate() to
produce full output before it consumes the input stream fully; e.g.
immediately before reading the end of stream marker.
Instead, detect corrupt input stream by feeding the input as long as
inflate() wants to without detecting a real error, and giving it an output
buffer that is one byte longer than necessary. If it touches the extra
byte, we know that the input stream is corrupt; otherwise inflate() will
notice the broken input stream by itself.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
sha1_file.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 4cc8939..f0907b8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1344,7 +1344,8 @@ unsigned long get_size_from_delta(struct packed_git *p,
off_t curpos)
{
const unsigned char *data;
- unsigned char delta_head[20], *in;
+ unsigned char delta_head[21], *in;
+ unsigned long expected_size = sizeof(delta_head) - 1;
z_stream stream;
int st;
@@ -1357,13 +1358,14 @@ unsigned long get_size_from_delta(struct packed_git *p,
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
+ if (!stream.avail_out)
+ break; /* the payload is larger than it should be! */
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
- stream.total_out < sizeof(delta_head));
+ stream.total_out < expected_size);
git_inflate_end(&stream);
- if ((st != Z_STREAM_END) && stream.total_out != sizeof(delta_head)) {
+ if ((st != Z_STREAM_END) &&
+ stream.total_out != expected_size) {
error("delta data unpack-initial failed");
return 0;
}
@@ -1589,15 +1591,15 @@ static void *unpack_compressed_entry(struct packed_git *p,
buffer[size] = 0;
memset(&stream, 0, sizeof(stream));
stream.next_out = buffer;
- stream.avail_out = size;
+ stream.avail_out = size + 1;
git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
+ if (!stream.avail_out)
+ break; /* the payload is larger than it should be! */
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
git_inflate_end(&stream);
--
1.6.5.1.107.gba912
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html