Liu Yubao <yubao.liu@xxxxxxxxx> wrote:
> diff --git a/sha1_file.c b/sha1_file.c
> index 6c0e251..efe6967 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1254,10 +1255,10 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
> /*
> * The type can be at most ten bytes (including the
> * terminating '\0' that we add), and is followed by
> - * a space.
> + * a space, at least one byte for size, and a '\0'.
> */
> i = 0;
> - for (;;) {
> + while (hdr < hdr_end - 2) {
> char c = *hdr++;
> if (c == ' ')
> break;
> @@ -1265,6 +1266,8 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
> if (i >= sizeof(type))
> return -1;
That first hunk I am citing is unnecessary, because of the lines
right above. All of the callers of this function pass in a buffer
that is at least 32 bytes in size; this loop aborts if it does not
find a ' ' within the first 10 bytes of the buffer. We'll never
access memory outside of the buffer during this loop.
So IMHO your first three hunks here aren't necessary.
> @@ -1275,7 +1278,7 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
> if (size > 9)
> return -1;
> if (size) {
> - for (;;) {
> + while (hdr < hdr_end - 1) {
> unsigned long c = *hdr - '0';
> if (c > 9)
> break;
OK, there's no promise here that we don't roll off the buffer.
This can be fixed in the caller, ensuring we always have the '\0'
at some point in the initial header buffer we were asked to parse:
diff --git a/sha1_file.c b/sha1_file.c
index 6c0e251..26c6ffb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1162,9 +1162,10 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
stream->next_in = map;
stream->avail_in = mapsize;
stream->next_out = buffer;
- stream->avail_out = bufsiz;
if (legacy_loose_object(map)) {
+ stream->avail_out = bufsiz - 1;
+ buffer[bufsiz - 1] = '\0';
inflateInit(stream);
return inflate(stream, 0);
}
@@ -1186,6 +1187,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
/* Set up the stream for the rest.. */
stream->next_in = map;
stream->avail_in = mapsize;
+ stream->avail_out = bufsiz;
inflateInit(stream);
/* And generate the fake traditional header */
--
Shawn.
--
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
