diff --git a/c/detools.c b/c/detools.c index 5d25489..6317128 100644 --- a/c/detools.c +++ b/c/detools.c @@ -1958,6 +1958,20 @@ static int in_place_process_size(struct detools_apply_patch_in_place_t *self_p, return (0); } +/* + * Returns true if given number of bytes at given from offset are all + * within the memory. The from offset comes from the patch, so it must + * be checked before it is used. + */ +static bool in_place_is_from_range_valid( + struct detools_apply_patch_in_place_t *self_p, + size_t offset, + size_t size) +{ + return ((offset <= self_p->memory_size) + && (size <= (self_p->memory_size - offset))); +} + static int in_place_process_data(struct detools_apply_patch_in_place_t *self_p, enum detools_apply_patch_state_t next_state) { @@ -1984,6 +1998,12 @@ static int in_place_process_data(struct detools_apply_patch_in_place_t *self_p, } if (next_state == detools_apply_patch_state_extra_size_t) { + if (!in_place_is_from_range_valid(self_p, + (size_t)self_p->segment.from_offset, + to_size)) { + return (-DETOOLS_CORRUPT_PATCH); + } + res = in_place_mem_read(self_p, &from[0], (size_t)self_p->segment.from_offset, @@ -2040,6 +2060,7 @@ static int in_place_process_adjustment(struct detools_apply_patch_in_place_t *se { int res; int offset; + size_t from_offset; res = patch_reader_unpack_size(&self_p->patch_reader, &offset); @@ -2054,7 +2075,15 @@ static int in_place_process_adjustment(struct detools_apply_patch_in_place_t *se res = in_place_next_step(self_p); self_p->state = detools_apply_patch_state_dfpatch_size_t; } else { - self_p->segment.from_offset += offset; + /* Unsigned, so it cannot overflow, and a negative result wraps + to a large value that is rejected as well. */ + from_offset = ((size_t)self_p->segment.from_offset + (size_t)offset); + + if (!in_place_is_from_range_valid(self_p, from_offset, 0)) { + return (-DETOOLS_CORRUPT_PATCH); + } + + self_p->segment.from_offset = (int)from_offset; self_p->state = detools_apply_patch_state_diff_size_t; } diff --git a/c/tst/test_fuzzer.c b/c/tst/test_fuzzer.c index 83fe3b7..e251e6b 100644 --- a/c/tst/test_fuzzer.c +++ b/c/tst/test_fuzzer.c @@ -153,6 +153,85 @@ static void test_one(const uint8_t *patch_buf_p, } } +static uint8_t memory[256]; + +static bool is_in_memory(uintptr_t address, size_t size) +{ + return ((address <= sizeof(memory)) && (size <= (sizeof(memory) - address))); +} + +static int mem_read(void *arg_p, void *dst_p, uintptr_t src, size_t size) +{ + (void)arg_p; + + if (!is_in_memory(src, size)) { + return (-1); + } + + memcpy(dst_p, &memory[src], size); + + return (0); +} + +static int mem_write(void *arg_p, uintptr_t dst, void *src_p, size_t size) +{ + (void)arg_p; + + if (!is_in_memory(dst, size)) { + return (-1); + } + + memcpy(&memory[dst], src_p, size); + + return (0); +} + +static int mem_erase(void *arg_p, uintptr_t addr, size_t size) +{ + (void)arg_p; + + if (!is_in_memory(addr, size)) { + return (-1); + } + + memset(&memory[addr], 0xff, size); + + return (0); +} + +static void test_one_in_place(const uint8_t *patch_buf_p, + size_t patch_size, + int process_res, + int finalize_res) +{ + struct detools_apply_patch_in_place_t apply_patch; + int res; + + res = detools_apply_patch_in_place_init(&apply_patch, + mem_read, + mem_write, + mem_erase, + NULL, + NULL, + patch_size, + NULL); + ASSERT_EQ(res, 0); + + res = detools_apply_patch_in_place_process(&apply_patch, + patch_buf_p, + patch_size); + + WITH_MESSAGE("Failed with '%s' (%d).", detools_error_as_string(res), res) { + ASSERT_EQ(res, process_res); + } + + res = detools_apply_patch_in_place_finalize(&apply_patch); + + WITH_MESSAGE("Failed with '%s' (%d).", detools_error_as_string(res), res) { + ASSERT_EQ(res, finalize_res); + } +} + TEST(size_overflow) { const uint8_t patch[] = { @@ -250,3 +329,56 @@ TEST(size_overflow_header) -DETOOLS_CORRUPT_PATCH_OVERFLOW, -DETOOLS_ALREADY_FAILED); } + +TEST(in_place_from_offset_before_memory) +{ + /* Memory size 8, segment size 4, shift size 4, from size 4 and + to size 1. The adjustment of -5 moves the from offset from 4 to + -1. */ + const uint8_t patch[] = { + 0x10, 0x08, 0x04, 0x04, 0x04, 0x01, 0x00, 0x00, 0x00, 0x45, + 0x01, 0x00, 0x00, 0x00 + }; + + test_one_in_place(&patch[0], + sizeof(patch), + -DETOOLS_CORRUPT_PATCH, + -DETOOLS_ALREADY_FAILED); +} + +TEST(in_place_from_read_past_memory) +{ + /* Memory size 8, segment size 4, shift size 4, from size 4 and + to size 4. The adjustment of 1 moves the from offset from 4 to + 5, and the diff then reads 4 bytes from there, past the end of + the memory. */ + const uint8_t patch[] = { + 0x10, 0x08, 0x04, 0x04, 0x04, 0x04, 0x00, 0x00, 0x00, 0x01, + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + }; + + test_one_in_place(&patch[0], + sizeof(patch), + -DETOOLS_CORRUPT_PATCH, + -DETOOLS_ALREADY_FAILED); +} + +TEST(in_place_from_offset_overflow) +{ + /* Memory size 8, segment size 4, shift size 4, from size 4 and + to size 4, followed by 17 empty diffs and extras, each with an + adjustment of 2^27 - 1. Together they overflow an int. */ + uint8_t patch[7 + 17 * 6]; + size_t i; + + memcpy(&patch[0], "\x10\x08\x04\x04\x04\x04\x00", 7); + + for (i = 7; i < sizeof(patch); i += 6) { + memcpy(&patch[i], "\x00\x00\xbf\xff\xff\x7f", 6); + } + + test_one_in_place(&patch[0], + sizeof(patch), + -DETOOLS_CORRUPT_PATCH, + -DETOOLS_ALREADY_FAILED); +}