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 index 7602f5bfaaa..069b94d28cc 100644 --- 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, int noffset; int verify; ulong len; + uint remaining; u32 *data; verify = env_get_yesno("verify"); + /* Format detection checks the FDT header, not just its magic. */ + if (size && size < sizeof(struct fdt_header)) { + puts("Truncated script image\n"); + return 1; + } + switch (genimg_get_format(buf)) { case IMAGE_FORMAT_LEGACY: if (!IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) { goto exit_image_format; } else { + if (size && size < sizeof(*hdr)) { + puts("Truncated script header\n"); + return 1; + } hdr = buf; if (!image_check_magic(hdr)) { @@ -1009,6 +1020,13 @@ int image_locate_script(void *buf, int size, const char *fit_uname, return 1; } + remaining = image_get_data_size(hdr); + if ((size && remaining > size - sizeof(*hdr)) || + remaining < 2 * sizeof(*data)) { + puts("Truncated script data\n"); + return 1; + } + if (verify) { if (!image_check_dcrc(hdr)) { puts("Bad data crc\n"); @@ -1035,7 +1053,17 @@ int image_locate_script(void *buf, int size, const char *fit_uname, * component, so seek past the zero-terminated sequence * of image lengths to get to the actual image data */ - while (*data++); + do { + if (remaining < sizeof(*data)) { + puts("Truncated script length table\n"); + return 1; + } + remaining -= sizeof(*data); + } while (*data++); + if (len > remaining) { + puts("Truncated script contents\n"); + return 1; + } } break; case IMAGE_FORMAT_FIT: diff --git a/include/image.h b/include/image.h index 5248ca98d41..efb0e0e68c7 100644 --- a/include/image.h +++ b/include/image.h @@ -840,7 +840,8 @@ static inline void fit_verity_free(struct bootm_headers *images) {} * image_locate_script() - Locate the raw script in an image * * @buf: Address of image - * @size: Size of image in bytes + * @size: Size of image in bytes, or 0 if unknown. Legacy length tables and + * script contents are also bounded by the data size in the image header. * @fit_uname: Node name of FIT image to read * @confname: Node name of FIT config to read * @datap: Returns pointer to raw script on success diff --git a/test/boot/image.c b/test/boot/image.c index 2c6d9dcbc22..c056b42df4a 100644 --- a/test/boot/image.c +++ b/test/boot/image.c @@ -6,7 +6,11 @@ * Written by Simon Glass <[email protected]> */ +#include <command.h> +#include <env.h> #include <image.h> +#include <mapmem.h> +#include <u-boot/crc.h> #include <test/ut.h> #include <linux/libfdt.h> #include "bootstd_common.h" @@ -100,3 +104,76 @@ static int test_image_phase(struct unit_test_state *uts) return 0; } BOOTSTD_TEST(test_image_phase, 0); + +/* Recompute checksums after changing the legacy length table or payload. */ +static void script_checksums(struct legacy_img_hdr *hdr) +{ + image_set_dcrc(hdr, crc32(0, (void *)image_get_data(hdr), + image_get_data_size(hdr))); + image_set_hcrc(hdr, 0); + image_set_hcrc(hdr, crc32(0, (void *)hdr, sizeof(*hdr))); +} + +/* Bound both known buffers and source's unknown-size images by their header. */ +static int image_legacy_script_bounds(struct unit_test_state *uts) +{ + struct { + struct legacy_img_hdr hdr; + u32 lengths[2]; + char script[4]; + } image; + struct legacy_img_hdr *hdr = &image.hdr; + char *data; + uint len; + int size, verify; + + if (!IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) + return -EAGAIN; + for (verify = 0; verify < 2; verify++) { + ut_assertok(env_set("verify", verify ? "yes" : "no")); + memset(&image, 0, sizeof(image)); + image_set_magic(hdr, IH_MAGIC); + image_set_type(hdr, IH_TYPE_SCRIPT); + image_set_size(hdr, sizeof(image) - sizeof(*hdr)); + image.lengths[0] = cpu_to_uimage(sizeof(image.script)); + memcpy(image.script, "true", sizeof(image.script)); + script_checksums(hdr); + ut_assertok(image_locate_script(&image, sizeof(image), NULL, + NULL, &data, &len)); + ut_asserteq(sizeof(image.script), len); + ut_asserteq_ptr(image.script, data); + ut_assertok(cmd_source_script(map_to_sysmem(&image), NULL, NULL)); + + /* A known size must bound reads before header/data CRC checks. */ + for (size = 1; size < sizeof(image); size++) + ut_asserteq(1, image_locate_script(&image, size, NULL, + NULL, &data, &len)); + + /* Missing terminator: do not walk into the script or beyond it. */ + memset(image.lengths, 0xff, sizeof(image) - sizeof(*hdr)); + script_checksums(hdr); + ut_asserteq(1, image_locate_script(&image, sizeof(image), NULL, + NULL, &data, &len)); + ut_asserteq(1, image_locate_script(&image, 0, NULL, NULL, + &data, &len)); + ut_asserteq(CMD_RET_FAILURE, + cmd_source_script(map_to_sysmem(&image), NULL, NULL)); + + /* A terminated table cannot name a script beyond the payload. */ + image.lengths[0] = cpu_to_uimage(sizeof(image.script) + 1); + image.lengths[1] = 0; + script_checksums(hdr); + ut_asserteq(1, image_locate_script(&image, 0, NULL, NULL, + &data, &len)); + + /* Reject a payload too short even for the length and terminator. */ + image_set_size(hdr, sizeof(u32)); + script_checksums(hdr); + ut_asserteq(1, image_locate_script(&image, 0, NULL, NULL, + &data, &len)); + } + + return 0; +} + +BOOTSTD_TEST(image_legacy_script_bounds, UTF_DM | UTF_SCAN_FDT);
