Skip to content

Commit 33b1d3f

Browse files
committed
[midx] Fix an undefined behavior (left-shift signed overflow)
There was a missing check to ensure that the `off64_t` (which is a signed value) didn't overflow when parsing it from the midx file. This shouldn't have huge repercusions since the parsed value is immediately validated afterwards, but then again, there is no such thing as "benign" undefined behavior. This change makes all the bitwise arithmetic happen with unsigned types and is only casted to `off64_t` until the very end. Thanks to Taotao Gu for finding and reporting this!
1 parent d8015d2 commit 33b1d3f

File tree

2 files changed

+8
-3
lines changed

2 files changed

+8
-3
lines changed
62 Bytes
Binary file not shown.

src/libgit2/midx.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,13 @@ int git_midx_parse(
225225
chunk_hdr = data + sizeof(struct git_midx_header);
226226
last_chunk = NULL;
227227
for (i = 0; i < hdr->chunks; ++i, chunk_hdr += 12) {
228-
chunk_offset = ((off64_t)ntohl(*((uint32_t *)(chunk_hdr + 4)))) << 32 |
229-
((off64_t)ntohl(*((uint32_t *)(chunk_hdr + 8))));
228+
uint32_t chunk_id = ntohl(*((uint32_t *)(chunk_hdr + 0)));
229+
uint64_t high_offset = ((uint64_t)ntohl(*((uint32_t *)(chunk_hdr + 4)))) & 0xffffffffu;
230+
uint64_t low_offset = ((uint64_t)ntohl(*((uint32_t *)(chunk_hdr + 8)))) & 0xffffffffu;
231+
232+
if (high_offset >= INT32_MAX)
233+
return midx_error("chunk offset out of range");
234+
chunk_offset = (off64_t)(high_offset << 32 | low_offset);
230235
if (chunk_offset < last_chunk_offset)
231236
return midx_error("chunks are non-monotonic");
232237
if (chunk_offset >= trailer_offset)
@@ -235,7 +240,7 @@ int git_midx_parse(
235240
last_chunk->length = (size_t)(chunk_offset - last_chunk_offset);
236241
last_chunk_offset = chunk_offset;
237242

238-
switch (ntohl(*((uint32_t *)(chunk_hdr + 0)))) {
243+
switch (chunk_id) {
239244
case MIDX_PACKFILE_NAMES_ID:
240245
chunk_packfile_names.offset = last_chunk_offset;
241246
last_chunk = &chunk_packfile_names;

0 commit comments

Comments
 (0)