Hi He,
On 2026-10-13T13:02:35, He Yong <[email protected]> wrote:
> image: Bound legacy script parsing by the image size
>
> The legacy script parser ignores its size argument and walks a length
> table until it finds a zero word. A malformed image can run this walk
> past the image buffer or declare a script longer than the remaining data.
>
> Check known header and payload sizes before CRC verification, bound the
> length-table walk by the declared payload, and check the script length.
> When the caller does not know the buffer size, as with source, still use
> the header's declared data size to bound the length table and contents.
>
> Add C regression tests for both verification settings and for source.
>
> Signed-off-by: He Yong <[email protected]>
>
> boot/image-board.c | 30 ++++++++++++++++++++-
> include/image.h | 3 ++-
> test/boot/image.c | 77
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 108 insertions(+), 2 deletions(-)
> diff --git a/boot/image-board.c b/boot/image-board.c
> @@ -988,15 +988,26 @@ int image_locate_script(void *buf, int size, const char
> *fit_uname,
> + /* Format detection checks the FDT header, not just its magic. */
> + if (size && size < sizeof(struct fdt_header)) {
> + puts("Truncated script image\n");
> + return 1;
> + }
Since this check applies to FIT images too, it would be worth passing
the size along in the FIT case as well, i.e. fit_check_format(fit_hdr,
size ? size : IMAGE_SIZE_INVAL) - with FIT_FULL_CHECK the FIT is then
bounded by the real buffer rather than by its own totalsize. This
could be a follow-up, but it may let patch 2 drop some of its own
fdt_check_full() handling. What do you think?
> diff --git a/test/boot/image.c b/test/boot/image.c
> @@ -100,3 +104,76 @@ static int test_image_phase(struct unit_test_state *uts)
> + for (verify = 0; verify < 2; verify++) {
> + ut_assertok(env_set("verify", verify ? "yes" : "no"));
The test leaves 'verify' set when it finishes, and if an assertion
fails in the first pass it stays at 'no' for later tests. Please can
you clear it at the end with env_set('verify', NULL)?
> diff --git a/test/boot/image.c b/test/boot/image.c
> @@ -100,3 +104,76 @@ static int test_image_phase(struct unit_test_state *uts)
> +
> +BOOTSTD_TEST(image_legacy_script_bounds, UTF_DM | UTF_SCAN_FDT);
This test doesn't use driver model or the devicetree, so the flags can
be 0, as with the other tests in this file. Please also drop the blank
line before BOOTSTD_TEST(), to match the rest of the file.
Regards,
Simon
