reftable/table: fix NULL pointer access when seeking to bogus offsets

When seeking an iterator to an arbitrary offset we may return a positive
value in case the offset points beyond the block. This makes sense when
iterating through multiple blocks of the same section, as the positive
value indicates to us that we're at the end of the table.

But when the offset originates from a section or index offset it is
supposed to point at a valid block, so an out-of-bounds value means that
the table is corrupt. Treating it as a normal end-of-iteration causes us
to silently report an empty section instead of surfacing the corruption,
and we are left with a partially-initialized block. This may later on
cause a NULL pointer exception:

  ==1486841==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55555598e02c bp 0x7fffffff4eb0 sp 0x7fffffff4e70 T0)
  ==1486841==The signal is caused by a READ memory access.
  ==1486841==Hint: address points to the zero page.
      #0 0x55555598e02c in reftable_block_type ./git/build/../reftable/block.c:392:9
      #1 0x55555598ee6e in block_iter_seek_key ./git/build/../reftable/block.c:536:35
      #2 0x5555559ae553 in table_iter_seek_linear ./git/build/../reftable/table.c:344:8
      #3 0x5555559adbff in table_iter_seek ./git/build/../reftable/table.c:450:9
      #4 0x5555559ada9c in table_iter_seek_void ./git/build/../reftable/table.c:460:9
      #5 0x555555992872 in reftable_iterator_seek_log_at ./git/build/../reftable/iter.c:281:9
      #6 0x555555992953 in reftable_iterator_seek_log ./git/build/../reftable/iter.c:287:9
      #7 0x55555583aa78 in test_reftable_table__seek_invalid_log_offset ./git/build/../t/unit-tests/u-reftable-table.c:257:20
      #8 0x5555557f684e in clar_run_test ./git/build/../t/unit-tests/clar/clar.c:335:3
      #9 0x5555557f2e69 in clar_run_suite ./git/build/../t/unit-tests/clar/clar.c:431:3
      #10 0x5555557f2882 in clar_test_run ./git/build/../t/unit-tests/clar/clar.c:636:4
      #11 0x5555557f375f in clar_test ./git/build/../t/unit-tests/clar/clar.c:687:11
      #12 0x5555557fa49d in cmd_main ./git/build/../t/unit-tests/unit-test.c:62:8
      #13 0x55555584cffa in main ./git/build/../common-main.c:9:11
      #14 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #15 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #16 0x555555694c24 in _start (./git/build/t/unit-tests+0x140c24)

  ==1486841==Register values:
  rax = 0x0000000000000000  rbx = 0x00007fffffff4ec0  rcx = 0x0000000000000000  rdx = 0x00007cfff6e2bd58
  rdi = 0x00007cfff6e2bd58  rsi = 0x00007bfff5da1020  rbp = 0x00007fffffff4eb0  rsp = 0x00007fffffff4e70
   r8 = 0x0000000000000000   r9 = 0x0000000000000002  r10 = 0x0000000000000000  r11 = 0x0000000000000017
  r12 = 0x00007fffffff5908  r13 = 0x0000000000000001  r14 = 0x00007ffff7ffd000  r15 = 0x0000555556056e90
  AddressSanitizer can not provide additional info.
  SUMMARY: AddressSanitizer: SEGV ./git/build/../reftable/block.c:392:9 in reftable_block_type
  ==1486841==ABORTING

Fix this by returning a proper error in `table_iter_seek_to()` when the
offset ranges beyond the block.

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 56362df0eda5a6b63fbbac5395c97ce05b822596..f4bc86a29deeacecdd6b06e0de086bb7d68de79e 100644 (file)
--- a/reftable/table.c
+++ b/reftable/table.c
@@ -242,6 +242,8 @@ static int table_iter_seek_to(struct table_iter *ti, uint64_t off, uint8_t typ)
        int err;
 
        err = table_init_block(ti->table, &ti->block, off, typ);
+       if (err > 0)
+               return REFTABLE_FORMAT_ERROR;
        if (err != 0)
                return err;
 

diff --git a/t/unit-tests/u-reftable-table.c b/t/unit-tests/u-reftable-table.c
index 14fae8b199c13c1b7b016ec651c70336102f450a..c7dca45e708c8f840b767645e505c4fc506f14e9 100644 (file)
--- a/t/unit-tests/u-reftable-table.c
+++ b/t/unit-tests/u-reftable-table.c
@@ -1,8 +1,11 @@
 #include "unit-test.h"
 #include "lib-reftable.h"
+#include "reftable/basics.h"
+#include "reftable/block.h"
 #include "reftable/blocksource.h"
 #include "reftable/constants.h"
 #include "reftable/iter.h"
+#include "reftable/reftable-error.h"
 #include "reftable/table.h"
 #include "strbuf.h"
 
@@ -199,3 +202,63 @@ void test_reftable_table__block_iterator(void)
        reftable_buf_release(&buf);
        reftable_free(records);
 }
+
+void test_reftable_table__seek_invalid_log_offset(void)
+{
+       struct reftable_ref_record refs[] = {
+               {
+                       .refname = (char *) "refs/heads/main",
+                       .value_type = REFTABLE_REF_VAL1,
+                       .value.val1 = { 42 },
+               },
+       };
+       struct reftable_log_record logs[] = {
+               {
+                       .refname = (char *) "refs/heads/main",
+                       .update_index = 1,
+                       .value_type = REFTABLE_LOG_UPDATE,
+                       .value.update = {
+                               .name = (char *) "user",
+                               .email = (char *) "user@example.com",
+                               .message = (char *) "message\n",
+                       },
+               },
+       };
+       struct reftable_block_source source = { 0 };
+       struct reftable_log_record log = { 0 };
+       struct reftable_iterator it = { 0 };
+       struct reftable_table *table;
+       struct reftable_buf buf = REFTABLE_BUF_INIT;
+       size_t fsize = footer_size(1);
+       uint8_t *footer;
+
+       cl_reftable_write_to_buf(&buf, refs, ARRAY_SIZE(refs),
+                                logs, ARRAY_SIZE(logs), NULL);
+
+       /*
+        * Corrupt the log section offset stored in the footer so that it points
+        * past the end of the table. The footer is checksummed, so we also have
+        * to recompute and rewrite the CRC.
+        */
+       footer = (uint8_t *) buf.buf + buf.len - fsize;
+       reftable_put_be64(footer + header_size(1) + 24, UINT64_MAX);
+       reftable_put_be32(footer + fsize - 4, crc32(0, footer, fsize - 4));
+
+       block_source_from_buf(&source, &buf);
+       cl_must_pass(reftable_table_new(&table, &source, "name"));
+
+       /*
+        * Seeking the log iterator must not crash even though the log section
+        * offset is bogus. As the offset points past the end of the table we
+        * know that the table is corrupt, so the seek must report a format
+        * error instead of pretending that the section is empty.
+        */
+       reftable_table_init_log_iterator(table, &it);
+       cl_assert_equal_i(reftable_iterator_seek_log(&it, ""),
+                         REFTABLE_FORMAT_ERROR);
+
+       reftable_log_record_release(&log);
+       reftable_iterator_destroy(&it);
+       reftable_table_decref(table);
+       reftable_buf_release(&buf);
+}