fit_image_get_data() checks that the external-data region fits in the addressable range and, for signed FITs, stays below FIT_SIGNATURE_MAX_SIZE. The SPL loader had no equivalent check, so a hostile data-offset or data-size could make the read offset wrap past ULONG_MAX, or the destination range wrap around the end of the address space.
Add the same check to load_simple_fit(), using the block-aligned read size and the source pointer the read actually uses. Signed-off-by: Anton Ivanov <[email protected]> Reviewed-by: Simon Glass <[email protected]> --- Changed since v3: the out-of-bounds message uses log_debug() instead of printf(). Code size, from buildman -S in the u-boot-gitlab-ci-runner container (gcc 14.2.0), one board per combination of CONFIG_SPL_LOAD_BLOCK (BLK), CONFIG_SPL_FIT_SIGNATURE (SIG) and bitness. No rodata or data change, so this is the cost of the checks themselves. SPL only; U-Boot proper is unchanged: BLK SIG bit board text all y n 32 am335x_evm +16 +16 y y 32 firefly-rk3288 +32 +32 n n 32 am335x_evm_spiboot +8 +8 n y 32 evb-ast2600 +40 +40 y n 64 imx8mm_evk +16 +16 y y 64 orangepi-5-ultra-rk3588 +40 +40 n n 64 r8a779g0_whitehawk 0 0 --- common/spl/spl_fit.c | 26 ++++++++++++++++++++++++++ test/image/spl_load.c | 23 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 65934674b6c..9da709fd79a 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -343,6 +343,32 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, src_ptr = map_sysmem(ALIGN(load_addr, ARCH_DMA_MINALIGN), len); length = len; + /* + * For non-signed FIT images, we can check that + * (read_offset + size) does not wrap and that + * (src_ptr + size) does not exceed the addressable range. + * For signed FITs, we can additionally check that + * (offset + len) doesn't exceed the allowed FIT image + * maximum size. + */ + if (size > ULONG_MAX - read_offset || + size > UINTPTR_MAX - (uintptr_t)src_ptr + /* + * #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE + * depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) + * is undefined when signing is disabled and referencing it + * here would fail to compile. + */ +#if CONFIG_IS_ENABLED(FIT_SIGNATURE) + || offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) || + len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset +#endif + ) { + log_debug("FIT external data is out of bounds (offset=%u, size=%u)\n", + offset, len); + return -EINVAL; + } + log_debug("reading from offset %x / %lx size %lx to %p: ", offset, read_offset, size, src_ptr); diff --git a/test/image/spl_load.c b/test/image/spl_load.c index 557362ba113..334ec77b18e 100644 --- a/test/image/spl_load.c +++ b/test/image/spl_load.c @@ -483,6 +483,29 @@ static int spl_test_fit_read_offset_overflow(struct unit_test_state *uts) } SPL_TEST(spl_test_fit_read_offset_overflow, 0); +/* + * A read whose end position (read_offset + size) wraps past the addressable + * range must be rejected. + */ +static int spl_test_fit_read_end_overflow(struct unit_test_state *uts) +{ + if (!image_supported(FIT_EXTERNAL)) + return -EAGAIN; + + /* + * The aligned external-data offset (0x2000 plus the size of the FIT + * itself) stays below the 0x2fff bytes remaining before ULONG_MAX, so + * read_offset passes the offset-wrap check above, but reading the + * SPL_TEST_DATA_SIZE bytes of data crosses past UINTPTR_MAX. + */ + spl_test_fit_offset = ULONG_MAX - 0x2fff; + + return check_fit_ext_prop(uts, FIT_DATA_OFFSET_PROP, 0x2000, 1, + spl_test_read_fit_offset, spl_test_fit_offset, + -EINVAL); +} +SPL_TEST(spl_test_fit_read_end_overflow, 0); + /* * LZMA is too complex to generate on the fly, so let's use some data I put in * the oven^H^H^H^H compressed earlier -- 2.55.0
