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>

diff --git a/reftable/table.c b/reftable/table.c
index f4bc86a29deeacecdd6b06e0de086bb7d68de79e..b4d3f9e211f8d283431d9f93d859d59c51936ced 100644 (file)
--- a/reftable/table.c
+++ b/reftable/table.c
@@ -562,6 +562,11 @@ int reftable_table_new(struct reftable_table **out,
                goto done;
        }
 
+       if (file_size < header_size(t->version) + footer_size(t->version)) {
+               err = REFTABLE_FORMAT_ERROR;
+               goto done;
+       }
+
        t->size = file_size - footer_size(t->version);
        t->source = *source;
        t->name = reftable_strdup(name);

diff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c
index c7dca45e708c8f840b767645e505c4fc506f14e9..28b0ef5258a3f06dcf46d58c0fcb6712beca1e5f 100644 (file)
--- a/t/unit-tests/u-reftable-table.c
+++ b/t/unit-tests/u-reftable-table.c
@@ -262,3 +262,31 @@ void test_reftable_table__seek_invalid_log_offset(void)
        reftable_table_decref(table);
        reftable_buf_release(&buf);
 }
+
+void test_reftable_table__new_with_truncated_table(void)
+{
+       struct reftable_ref_record refs[] = {
+               {
+                       .refname = (char *) "refs/heads/main",
+                       .value_type = REFTABLE_REF_VAL1,
+                       .value.val1 = { 42 },
+               },
+       };
+       struct reftable_block_source source = { 0 };
+       struct reftable_table *table;
+       struct reftable_buf buf = REFTABLE_BUF_INIT;
+
+       cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs), NULL, 0, NULL);
+
+       /*
+        * Truncate the table so that it is large enough to read the header, but
+        * too small to also contain the footer.
+        */
+       buf.len = footer_size(1) - 1;
+       block_source_from_buf(&source, &buf);
+
+       cl_assert_equal_i(reftable_table_new(&table, &source, "name"),
+                         REFTABLE_FORMAT_ERROR);
+
+       reftable_buf_release(&buf);
+}