From cf469ab0783da6783f89a8e31c213f19fdf38dba Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 15 Feb 2021 17:08:10 -0700 Subject: [PATCH] image: Add an option to do a full check of the FIT Some strange modifications of the FIT can introduce security risks. Add an option to check it thoroughly, using libfdt's fdt_check_full() function. Enable this by default if signature verification is enabled. CVE-2021-27097 Signed-off-by: Simon Glass Reported-by: Bruce Monroe Reported-by: Arie Haenel Reported-by: Julien Lenoir --- Kconfig | 19 ++++++++++++ common/image-fit.c | 20 ++++++++++++- include/linux/libfdt.h | 2 ++ lib/libfdt/fdt_ro.c | 65 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/Kconfig b/Kconfig index b62bcdbccf1e..0165ca33c5d1 100644 --- a/Kconfig +++ b/Kconfig @@ -318,11 +318,21 @@ config FIT_ENABLE_SHA512_SUPPORT SHA512 checksum is a 512-bit (64-byte) hash value used to check that the image contents have not been corrupted. +config FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + default y + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. + config FIT_SIGNATURE bool "Enable signature verification of FIT uImages" depends on DM select HASH select RSA + select FIT_FULL_CHECK help This option enables signature verification of FIT uImages, using a hash signed and verified using RSA. If @@ -398,6 +408,14 @@ config SPL_FIT_PRINT help Support printing the content of the fitImage in a verbose manner in SPL. +config SPL_FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. + config SPL_FIT_SIGNATURE bool "Enable signature verification of FIT firmware within SPL" depends on SPL_DM @@ -405,6 +423,7 @@ config SPL_FIT_SIGNATURE select SPL_CRYPTO_SUPPORT select SPL_HASH_SUPPORT select SPL_RSA + select SPL_FIT_FULL_CHECK config SPL_LOAD_FIT bool "Enable SPL loading U-Boot as a FIT" diff --git a/common/image-fit.c b/common/image-fit.c index 124d8895cffd..b1926d8b53f8 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -15,7 +15,6 @@ #include #else #include -#include #include #include #include @@ -26,12 +25,15 @@ DECLARE_GLOBAL_DATA_PTR; #include #include +#include #include #include #include #include #include +#define log_debug(fmt, args...) debug(fmt, ##args) + /*****************************************************************************/ /* New uImage format routines */ /*****************************************************************************/ @@ -1487,6 +1489,22 @@ int fit_check_format(const void *fit, ulong size) return -ENODATA; } } + + if (CONFIG_IS_ENABLED(FIT_FULL_CHECK)) { + /* + * If we are not given the size, make do wtih calculating it. + * This is not as secure, so we should consider a flag to + * control this. + */ + if (size == IMAGE_SIZE_INVAL) + size = fdt_totalsize(fit); + ret = fdt_check_full(fit, size); + + if (ret) { + log_debug("FIT check error %d\n", ret); + return -EINVAL; + } + } /* mandatory subimages parent '/images' node */ if (fdt_path_offset(fit, FIT_IMAGES_PATH) < 0) { diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h index eeb2344971f3..29c997ada398 100644 --- a/include/linux/libfdt.h +++ b/include/linux/libfdt.h @@ -305,6 +305,8 @@ int fdt_next_region(const void *fdt, */ int fdt_add_alias_regions(const void *fdt, struct fdt_region *region, int count, int max_regions, struct fdt_region_state *info); + +int fdt_check_full(const void *fdt, size_t bufsize); #endif /* SWIG */ extern struct fdt_header *working_fdt; /* Pointer to the working fdt */ diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index b6ca4e0b0c30..dfbeb2c21a85 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -680,3 +680,68 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset, return offset; /* error from fdt_next_node() */ } + +#define INT_MAX ((int)(~0U>>1)) + +int fdt_check_full(const void *fdt, size_t bufsize) +{ + int err; + int num_memrsv; + int offset, nextoffset = 0; + uint32_t tag; + unsigned depth = 0; + const void *prop; + const char *propname; + + if (bufsize < FDT_V1_SIZE) + return -FDT_ERR_TRUNCATED; + err = fdt_check_header(fdt); + if (err != 0) + return err; + if (bufsize < fdt_totalsize(fdt)) + return -FDT_ERR_TRUNCATED; + + num_memrsv = fdt_num_mem_rsv(fdt); + if (num_memrsv < 0) + return num_memrsv; + + while (1) { + offset = nextoffset; + tag = fdt_next_tag(fdt, offset, &nextoffset); + + if (nextoffset < 0) + return nextoffset; + + switch (tag) { + case FDT_NOP: + break; + + case FDT_END: + if (depth != 0) + return -FDT_ERR_BADSTRUCTURE; + return 0; + + case FDT_BEGIN_NODE: + depth++; + if (depth > INT_MAX) + return -FDT_ERR_BADSTRUCTURE; + break; + + case FDT_END_NODE: + if (depth == 0) + return -FDT_ERR_BADSTRUCTURE; + depth--; + break; + + case FDT_PROP: + prop = fdt_getprop_by_offset(fdt, offset, &propname, + &err); + if (!prop) + return err; + break; + + default: + return -FDT_ERR_INTERNAL; + } + } +} -- 2.17.1