Hi Anton,
On 2026-07-30T15:03:40, Anton Ivanov <[email protected]> wrote:
> spl: fit: Harden external-data offset and size arithmetic
>
> The data-offset, data-position and data-size FIT properties are
> excluded from the configuration signature, so they are attacker
> controlled even when signature verification succeeds. The offset and
> size arithmetic in load_simple_fit() can wrap on hostile values:
>
> - adding the external-data base offset to data-offset can wrap past
> UINT32_MAX,
> - get_aligned_image_size() adds the block-alignment overhead and
> rounds up to the block length, which can wrap past ULONG_MAX,
> - adding the FIT's device offset to the aligned external-data offset
> can wrap past ULONG_MAX.
>
> Make get_aligned_image_size() return the aligned size through an out
> parameter and fail with -EOVERFLOW when the computation would wrap,
> check the two offset additions explicitly, and compare the
> block-aligned size (the amount info->read() actually transfers)
> against max_size before reading. Do the same for the FIT header read
> in spl_simple_fit_read().
> [...]
>
> common/spl/spl_fit.c | 98 +++++++++++++++++++++++++++++----------------------
> test/image/spl_load.c | 57 ++++++++++++++++++++++++++++++
> 2 files changed, 112 insertions(+), 43 deletions(-)
Just to flag - first_loading_rbf_to_buffer() has the same 'rbf_offset
+= ((fdt_totalsize(buffer_p) + 3) & ~3)' pattern...out of scope but
the maintainer may wish to take a look.
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -700,12 +700,8 @@ def test_vboot_ext_data_bounds(ubman):
> fd.write(500 * b'\0')
>
> testcases = [
> - ('negative data-position',
> - {'data-position': 0xffffffff}, 'Invalid external data position'),
> - ('negative data-offset',
> + ('invalid data-offset',
> {'data-offset': 0xffffffff}, 'Invalid external data offset'),
> - ('negative data-size',
> - {'data-size': 0xffffffff}, 'Invalid external data size'),
> ('off-bounds data-position',
> {'data-position': 0x7fffffff}, 'FIT external data is out of
> bounds'),
> ('off-bounds data-offset',
The 0xffffffff data-position and data-size cases are dropped rather
than repointed at the 'out of bounds' message they now hit. Please
keep the 0xffffffff coverage so a regression that stopped rejecting a
UINT32_MAX property would still be caught - a rename to 'invalid
data-position'/'invalid data-size' with the new expected message would
do it.
Is there any code-size impact for this series?
Regards,
Simon
