Re: [PATCH v2 07/12] reftable/block: fix OOB read with bogus block size

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jul 03, 2026 at 11:28:21AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/unit-tests/u-reftable-block.c b/t/unit-tests/u-reftable-block.c
> > index 088162483e..43b9d5fb59 100644
> > --- a/t/unit-tests/u-reftable-block.c
> > +++ b/t/unit-tests/u-reftable-block.c
> > @@ -497,3 +497,36 @@ void test_reftable_block__corrupt_log_block_size(void)
> >  	reftable_block_release(&block);
> >  	reftable_buf_release(&data);
> >  }
> > +
> > +void test_reftable_block__corrupt_block_size(void)
> > +{
> > +	struct reftable_block_source source = { 0 };
> > +	struct reftable_record rec = {
> > +		.type = REFTABLE_BLOCK_TYPE_REF,
> > +		.u.ref = {
> > +			.value_type = REFTABLE_REF_VAL1,
> > +			.refname = (char *) "refs/heads/main",
> > +		},
> > +	};
> > +	struct reftable_block block = { 0 };
> > +	struct reftable_buf data = REFTABLE_BUF_INIT;
> > +
> > +	cl_reftable_write_block(&data, REFTABLE_BLOCK_TYPE_REF, &rec, 1);
> > +
> > +	/*
> > +	 * The block size is stored as a big-endian 24-bit integer right after
> > +	 * the one-byte block type at the start of the block. Corrupt it to
> > +	 * claim a size that is larger than the data we actually have. Reading
> > +	 * the restart count and restart table relative to such a bogus block
> > +	 * size must not access out-of-bounds memory.
> > +	 */
> > +	reftable_put_be24((uint8_t *) data.buf + 1, 0xffffff);
> 
> Same here, would it make sense to write a size that's `+1` too much?
> 
> 	uint8_t *p = (uint8_t *)data.buf + 1;
> 	uint32_t block_size = reftable_get_be24(p);
> 	cl_assert_equal_i(block_size, 47);
> 	reftable_put_be24(p, block_size + 1);

In this case it'd work indeed though, and I agree that this is a
sensible change.

Patrick