Skip to content

Commit f7f0a87

Browse files
committed
Merge branch 'jc/verify-loose-object-header' into maint
Codepaths that read from an on-disk loose object were too loose in validating what they are reading is a proper object file and sometimes read past the data they read from the disk, which has been corrected. H/t to Gustavo Grieco for reporting. * jc/verify-loose-object-header: unpack_sha1_header(): detect malformed object header streaming: make sure to notice corrupt object
2 parents 0bc409d + d21f842 commit f7f0a87

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

sha1_file.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,9 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
15721572
return used;
15731573
}
15741574

1575-
int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)
1575+
static int unpack_sha1_short_header(git_zstream *stream,
1576+
unsigned char *map, unsigned long mapsize,
1577+
void *buffer, unsigned long bufsiz)
15761578
{
15771579
/* Get the data stream */
15781580
memset(stream, 0, sizeof(*stream));
@@ -1585,13 +1587,31 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
15851587
return git_inflate(stream, 0);
15861588
}
15871589

1590+
int unpack_sha1_header(git_zstream *stream,
1591+
unsigned char *map, unsigned long mapsize,
1592+
void *buffer, unsigned long bufsiz)
1593+
{
1594+
int status = unpack_sha1_short_header(stream, map, mapsize,
1595+
buffer, bufsiz);
1596+
1597+
if (status < Z_OK)
1598+
return status;
1599+
1600+
/* Make sure we have the terminating NUL */
1601+
if (!memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
1602+
return -1;
1603+
return 0;
1604+
}
1605+
15881606
static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
15891607
unsigned long mapsize, void *buffer,
15901608
unsigned long bufsiz, struct strbuf *header)
15911609
{
15921610
int status;
15931611

1594-
status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
1612+
status = unpack_sha1_short_header(stream, map, mapsize, buffer, bufsiz);
1613+
if (status < Z_OK)
1614+
return -1;
15951615

15961616
/*
15971617
* Check if entire header is unpacked in the first iteration.
@@ -1682,6 +1702,8 @@ static int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
16821702
*/
16831703
for (;;) {
16841704
char c = *hdr++;
1705+
if (!c)
1706+
return -1;
16851707
if (c == ' ')
16861708
break;
16871709
type_len++;

streaming.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,17 +337,17 @@ static open_method_decl(loose)
337337
st->u.loose.mapped = map_sha1_file(sha1, &st->u.loose.mapsize);
338338
if (!st->u.loose.mapped)
339339
return -1;
340-
if (unpack_sha1_header(&st->z,
341-
st->u.loose.mapped,
342-
st->u.loose.mapsize,
343-
st->u.loose.hdr,
344-
sizeof(st->u.loose.hdr)) < 0) {
340+
if ((unpack_sha1_header(&st->z,
341+
st->u.loose.mapped,
342+
st->u.loose.mapsize,
343+
st->u.loose.hdr,
344+
sizeof(st->u.loose.hdr)) < 0) ||
345+
(parse_sha1_header(st->u.loose.hdr, &st->size) < 0)) {
345346
git_inflate_end(&st->z);
346347
munmap(st->u.loose.mapped, st->u.loose.mapsize);
347348
return -1;
348349
}
349350

350-
parse_sha1_header(st->u.loose.hdr, &st->size);
351351
st->u.loose.hdr_used = strlen(st->u.loose.hdr) + 1;
352352
st->u.loose.hdr_avail = st->z.total_out;
353353
st->z_state = z_used;

0 commit comments

Comments
 (0)