Skip to content

Commit f4e163f

Browse files
pks-tgitster
authored andcommitted
reftable/table: fix OOB read on truncated table
When opening a table we compute the size of its data section by subtracting the footer size from the file size. We do not verify that the file is actually large enough to contain both the header and the footer though. With a truncated table the subtraction can thus underflow, causing us to read the footer out of bounds: SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/pks/Development/git/build/t/unit-tests+0x2479a4) in __asan_memcpy Shadow bytes around the buggy address: 0x7ccff6e0de80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x7ccff6e0df00: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa 0x7ccff6e0df80: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x7ccff6e0e000: fd fd fd fd fa fa fa fa fa fa fa fa fd fd fd fd 0x7ccff6e0e080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa =>0x7ccff6e0e100: fa fa fa fa fa[fa]00 00 00 00 00 00 00 00 00 00 0x7ccff6e0e180: 00 00 00 00 00 00 00 04 fa fa fa fa fa fa fa fa 0x7ccff6e0e200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ccff6e0e280: 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7ccff6e0e300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7ccff6e0e380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==1500371==ABORTING Verify that the file is large enough to contain both the header and the footer before computing the table size. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 83b2233 commit f4e163f

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

reftable/table.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,11 @@ int reftable_table_new(struct reftable_table **out,
562562
goto done;
563563
}
564564

565+
if (file_size < header_size(t->version) + footer_size(t->version)) {
566+
err = REFTABLE_FORMAT_ERROR;
567+
goto done;
568+
}
569+
565570
t->size = file_size - footer_size(t->version);
566571
t->source = *source;
567572
t->name = reftable_strdup(name);

t/unit-tests/u-reftable-table.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,31 @@ void test_reftable_table__seek_invalid_log_offset(void)
262262
reftable_table_decref(table);
263263
reftable_buf_release(&buf);
264264
}
265+
266+
void test_reftable_table__new_with_truncated_table(void)
267+
{
268+
struct reftable_ref_record refs[] = {
269+
{
270+
.refname = (char *) "refs/heads/main",
271+
.value_type = REFTABLE_REF_VAL1,
272+
.value.val1 = { 42 },
273+
},
274+
};
275+
struct reftable_block_source source = { 0 };
276+
struct reftable_table *table;
277+
struct reftable_buf buf = REFTABLE_BUF_INIT;
278+
279+
cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs), NULL, 0, NULL);
280+
281+
/*
282+
* Truncate the table so that it is large enough to read the header, but
283+
* too small to also contain the footer.
284+
*/
285+
buf.len = footer_size(1) - 1;
286+
block_source_from_buf(&source, &buf);
287+
288+
cl_assert_equal_i(reftable_table_new(&table, &source, "name"),
289+
REFTABLE_FORMAT_ERROR);
290+
291+
reftable_buf_release(&buf);
292+
}

0 commit comments

Comments
 (0)