Re: [PATCH v2 06/12] reftable/block: fix OOB write with bogus inflated log size
From: Toon Claes <hidden>
Date: 2026-07-03 09:23:45
Patrick Steinhardt [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/t/unit-tests/u-reftable-block.c b/t/unit-tests/u-reftable-block.c index f4e926ce3a..088162483e 100644 --- a/t/unit-tests/u-reftable-block.c +++ b/t/unit-tests/u-reftable-block.c@@ -465,3 +465,35 @@ void test_reftable_block__iterator(void) reftable_block_release(&block); reftable_buf_release(&data); } + +void test_reftable_block__corrupt_log_block_size(void) +{ + struct reftable_block_source source = { 0 }; + struct reftable_record rec = { + .type = REFTABLE_BLOCK_TYPE_LOG, + .u.log = { + .refname = (char *) "refs/heads/main", + .update_index = 1, + .value_type = REFTABLE_LOG_UPDATE, + }, + }; + struct reftable_block block = { 0 }; + struct reftable_buf data = REFTABLE_BUF_INIT; + + cl_reftable_write_block(&data, REFTABLE_BLOCK_TYPE_LOG, &rec, 1); + + /* + * Log blocks store their inflated size as a big-endian 24-bit integer + * right after the one-byte block type. Rewrite it to claim a size that + * is smaller than the block header. + */ + reftable_put_be24((uint8_t *) data.buf + 1, 1);
Can I suggest to make this test a bit more strict: /* * Log blocks store their inflated size as a big-endian 24-bit integer * right after the one-byte block type. * First sanity check if the expected value is actually there, * then rewrite it to claim a size that is smaller than the block header. */ void *p = (uint8_t *) data.buf + 1; uint32_t block_size = reftable_get_be24(p); cl_assert_equal_i(block_size, 82); reftable_put_be24(p, 5); So first do a sanity check to see if the current block_size is what we expect, this helps us ensure we are actually reading the correct point in the buffers. Or does this depend on the architecture? Then write a size that's just below what the expected value is. By using a value that's just too small, maybe we can avoid off-by-one errors?
+ + block_source_from_buf(&source, &data); + cl_assert_equal_i(reftable_block_init(&block, &source, 0, 0, data.len, + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_LOG), + REFTABLE_FORMAT_ERROR); + + reftable_block_release(&block); + reftable_buf_release(&data); +}
-- Cheers, Toon