load_simple_fit() loads an image stored as external data by reading it from the boot device with a transfer sized from the FIT data-size property. That property is listed in exc_prop[] in image-fit-sig.c, so it is excluded from the configuration signature and stays under the control of anyone able to modify the boot medium even when CONFIG_SPL_FIT_SIGNATURE is enabled. The read happens before fit_image_verify_with_data() checks the image hash, so an inflated data-size overruns the destination before the corruption can be detected. The device-tree overlay path is the sharpest case, because there the destination is a fixed CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY_BUF_SZ heap buffer.
Pass the size of the destination into load_simple_fit() and reject an image whose data does not fit before the read is issued. The data-size property is read into an int, so a value with bit 31 set arrives as a negative number, and comparing it as an unsigned quantity rejects that case as well. The block-aligned transfer length is checked too, because info->read() moves data-size rounded up to the device block length. Callers that load into a memory region pass CONFIG_SYS_BOOTM_LEN, the limit spl_parse_legacy_validate() already applies to legacy images. The overlay path passes the size of its temporary buffer. Signed-off-by: Aristo Chen <[email protected]> --- common/spl/spl_fit.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 46ebcabe56a..224fd97d680 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -204,6 +204,9 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size, * If the FIT node does not contain a "load" (address) property, * the image gets loaded to the address pointed to by the * load_addr member in this struct, if load_addr is not 0 + * @max_size: maximum number of bytes that may be written to the + * destination; an image whose data exceeds this is rejected + * before it is read from the device * * Return: 0 on success, -EBADSLT if this image is not the correct phase * (for CONFIG_BOOTMETH_VBE_SIMPLE_FW), or another negative error number on @@ -211,7 +214,7 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size, */ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, const struct spl_fit_info *ctx, int node, - struct spl_image_info *image_info) + struct spl_image_info *image_info, ulong max_size) { int offset; size_t length; @@ -291,6 +294,18 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, return 0; } + /* + * data-size is excluded from the configuration signature (it + * is in the exc_prop[] list in image-fit-sig.c), so it remains + * attacker-controlled even after fit_config_verify() succeeds. + * The image hash is only verified after the device read below, + * so an oversized data-size has to be rejected here. The value + * is stored into an int, so one with bit 31 set arrives as a + * negative number; the unsigned compare rejects that too. + */ + if ((ulong)len > max_size) + goto too_big; + if (spl_decompression_enabled() && (image_comp == IH_COMP_GZIP || image_comp == IH_COMP_LZMA)) src_ptr = map_sysmem(ALIGN(CONFIG_SYS_LOAD_ADDR, ARCH_DMA_MINALIGN), len); @@ -302,6 +317,15 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, size = get_aligned_image_size(info, length, offset); read_offset = fit_offset + get_aligned_image_offset(info, offset); + + /* The block-aligned read size must also fit the destination */ + if (size > max_size) { +too_big: + printf("%s: FIT image too large (max %lu)\n", + __func__, max_size); + return -EFBIG; + } + log_debug("reading from offset %x / %lx size %lx to %p: ", offset, read_offset, size, src_ptr); @@ -427,7 +451,8 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image, spl_image->fdt_addr = map_sysmem(image_info.load_addr, size); memcpy(spl_image->fdt_addr, gd->fdt_blob, size); } else { - ret = load_simple_fit(info, offset, ctx, node, &image_info); + ret = load_simple_fit(info, offset, ctx, node, &image_info, + CONFIG_SYS_BOOTM_LEN); if (ret < 0) return ret; @@ -479,7 +504,8 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image, } image_info.load_addr = (ulong)tmpbuffer; ret = load_simple_fit(info, offset, ctx, node, - &image_info); + &image_info, + CONFIG_SPL_LOAD_FIT_APPLY_OVERLAY_BUF_SZ); if (ret == -EBADSLT) continue; else if (ret < 0) @@ -687,7 +713,8 @@ static int spl_fit_load_fpga(struct spl_fit_info *ctx, warn_deprecated("'fpga' property in config node. Use 'loadables'"); /* Load the image and set up the fpga_image structure */ - ret = load_simple_fit(info, offset, ctx, node, &fpga_image); + ret = load_simple_fit(info, offset, ctx, node, &fpga_image, + CONFIG_SYS_BOOTM_LEN); if (ret) { printf("%s: Cannot load the FPGA: %i\n", __func__, ret); return ret; @@ -849,7 +876,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, } /* Load the image and set up the spl_image structure */ - ret = load_simple_fit(info, offset, &ctx, node, spl_image); + ret = load_simple_fit(info, offset, &ctx, node, spl_image, + CONFIG_SYS_BOOTM_LEN); if (ret) return ret; @@ -890,7 +918,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, continue; image_info.load_addr = 0; - ret = load_simple_fit(info, offset, &ctx, node, &image_info); + ret = load_simple_fit(info, offset, &ctx, node, &image_info, + CONFIG_SYS_BOOTM_LEN); if (ret < 0 && ret != -EBADSLT) { printf("%s: can't load image loadables index %d (ret = %d)\n", __func__, index, ret); -- 2.43.0
