Fix potential OOB-read in delta diff

[danielinux]

F/434 - potential out-of-bounds read while creating the patch

Also includes minor unrelated fix:

F/436: Added missing parenthesis around macro

[Copilot]

Pull request overview

Fixes a potential out-of-bounds read in wb_diff() when extending matches to the end of the input buffers, and adds unit tests to reproduce those edge cases. Also includes a small macro correctness tweak.

Changes:

• Add bounds checks in wb_diff() match-extension loops to prevent OOB reads.
• Add new unit tests covering “match extends to end of src_b” scenarios.
• Fix a macro precedence issue by adding missing parentheses.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
tools/unit-tests/unit-delta.c	Adds regression tests for the OOB scenario; refactors buffer initialization helper.
src/delta.c	Adds bounds checks to prevent match-extension loops from reading past buffer ends.
src/libwolfboot.c	Wraps a macro expression in parentheses to avoid precedence issues.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

wb_diff_get_sector_size() returns an int, but the test stores it in a uint32_t. If the function returns a negative value on error, it will wrap to a large unsigned number, making the ck_assert_uint_gt pass and potentially triggering huge allocations. Store the return value in an int (or size_t after validating it’s positive), assert it’s > 0 / > BLOCK_HDR_SIZE using signed asserts, then convert for allocation sizes.

Suggested change

	uint32_t sector_size;
	int ret;
	sector_size = wb_diff_get_sector_size();
	ck_assert_uint_gt(sector_size, BLOCK_HDR_SIZE);
	src_a = calloc(1, sector_size + BLOCK_HDR_SIZE);
	src_b = calloc(1, sector_size + BLOCK_HDR_SIZE + 1);
	ck_assert_ptr_nonnull(src_a);
	ck_assert_ptr_nonnull(src_b);
	ret = wb_diff_init(&diff_ctx, src_a, sector_size + BLOCK_HDR_SIZE,
	src_b, sector_size + BLOCK_HDR_SIZE + 1);
	int sector_size;
	int ret;
	sector_size = wb_diff_get_sector_size();
	ck_assert_int_gt(sector_size, (int)BLOCK_HDR_SIZE);
	src_a = calloc(1, (size_t)sector_size + BLOCK_HDR_SIZE);
	src_b = calloc(1, (size_t)sector_size + BLOCK_HDR_SIZE + 1);
	ck_assert_ptr_nonnull(src_a);
	ck_assert_ptr_nonnull(src_b);
	ret = wb_diff_init(&diff_ctx, src_a, (size_t)sector_size + BLOCK_HDR_SIZE,
	src_b, (size_t)sector_size + BLOCK_HDR_SIZE + 1);

[Copilot]

initialize_buffers() now takes a size parameter, but it still writes to fixed indices (510, 1022) unconditionally. This will go out of bounds if the helper is ever called with a smaller buffer size. Either guard these writes (e.g., only set them when size > 1022) or compute the marker positions relative to size.

[Copilot]

initialize_buffers() now takes a size parameter, but it still writes to fixed indices (510, 1022) unconditionally. This will go out of bounds if the helper is ever called with a smaller buffer size. Either guard these writes (e.g., only set them when size > 1022) or compute the marker positions relative to size.

[Copilot]

The new bounds checks rely on casting pointer differences to uintptr_t, which is harder to reason about and can mask sign/width issues (and tends to trip static analysis). Prefer expressing bounds in terms of end pointers (e.g., pa < ctx->src_a + ctx->size_a and pb < ctx->src_b + pb_end) or use size_t for offsets derived from pointer subtraction and compare against (size_t)ctx->size_*.

[Copilot]

The new bounds checks rely on casting pointer differences to uintptr_t, which is harder to reason about and can mask sign/width issues (and tends to trip static analysis). Prefer expressing bounds in terms of end pointers (e.g., pa < ctx->src_a + ctx->size_a and pb < ctx->src_b + pb_end) or use size_t for offsets derived from pointer subtraction and compare against (size_t)ctx->size_*.