From 8d4681a526451a22408548bb8fcd2f80b74195a6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 15 Feb 2021 17:08:12 -0700 Subject: [PATCH] [PATCH] image: Check for unit addresses in FITs Using unit addresses in a FIT is a security risk. Add a check for this and disallow it. CVE-2021-27138 Signed-off-by: Simon Glass Reported-by: Bruce Monroe Reported-by: Arie Haenel Reported-by: Julien Lenoir The test part has not been patched. It would require these patches as well: https://github.com/u-boot/u-boot/commit/fafafacb470b345f2f41b86e4633ef91a7c5ed23 https://github.com/u-boot/u-boot/commit/d5f3aadacbc63df3b690d6fd9f0aa3f575b43356 Also, remove the broken test in test/image/test-imagetools.sh (thanks to jspricke for the hint): https://salsa.debian.org/debian/u-boot/-/blob/debian/latest/debian/patches/disable-fit-image-tests?ref_type=heads https://lists.denx.de/pipermail/u-boot/2021-March/445431.html Reviewed-By: Daniel Leidert Origin: https://github.com/u-boot/u-boot/commit/3f04db891a353f4b127ed57279279f851c6b4917 Bug: https://github.com/advisories/GHSA-grrh-mjp7-g52c Bug-Debian: https://bugs.debian.org/983269 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-27138 Bug-Freexian-Security: https://deb.freexian.com/extended-lts/tracker/CVE-2021-27138 Gbp-Pq: Name CVE-2021-27138-2.patch --- common/image-fit.c | 56 ++++++++++++++++++++++++++++++++--- test/image/test-imagetools.sh | 12 -------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/common/image-fit.c b/common/image-fit.c index b9edcc7a4..1b475224e 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1552,6 +1552,34 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) return (comp == image_comp); } +/** + * fdt_check_no_at() - Check for nodes whose names contain '@' + * + * This checks the parent node and all subnodes recursively + * + * @fit: FIT to check + * @parent: Parent node to check + * @return 0 if OK, -EADDRNOTAVAIL is a node has a name containing '@' + */ +static int fdt_check_no_at(const void *fit, int parent) +{ + const char *name; + int node; + int ret; + + name = fdt_get_name(fit, parent, NULL); + if (!name || strchr(name, '@')) + return -EADDRNOTAVAIL; + + fdt_for_each_subnode(node, fit, parent) { + ret = fdt_check_no_at(fit, node); + if (ret) + return ret; + } + + return 0; +} + int fit_check_format(const void *fit, ulong size) { int ret; @@ -1573,10 +1601,27 @@ int fit_check_format(const void *fit, ulong size) if (size == IMAGE_SIZE_INVAL) size = fdt_totalsize(fit); ret = fdt_check_full(fit, size); + if (ret) + ret = -EINVAL; + + /* + * U-Boot stopped using unit addressed in 2017. Since libfdt + * can match nodes ignoring any unit address, signature + * verification can see the wrong node if one is inserted with + * the same name as a valid node but with a unit address + * attached. Protect against this by disallowing unit addresses. + */ + if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) { + ret = fdt_check_no_at(fit, 0); + if (ret) { + log_debug("FIT check error %d\n", ret); + return ret; + } + } if (ret) { log_debug("FIT check error %d\n", ret); - return -EINVAL; + return ret; } } @@ -1940,10 +1985,13 @@ int fit_image_load(bootm_headers_t *images, ulong addr, printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr); bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT); - if (fit_check_format(fit, IMAGE_SIZE_INVAL)) { - printf("Bad FIT %s image format!\n", prop_name); + ret = fit_check_format(fit, IMAGE_SIZE_INVAL); + if (ret) { + printf("Bad FIT %s image format! (err=%d)\n", prop_name, ret); + if (CONFIG_IS_ENABLED(FIT_SIGNATURE) && ret == -EADDRNOTAVAIL) + printf("Signature checking prevents use of unit addresses (@) in nodes\n"); bootstage_error(bootstage_id + BOOTSTAGE_SUB_FORMAT); - return -ENOEXEC; + return ret; } bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT_OK); if (fit_uname) { diff --git a/test/image/test-imagetools.sh b/test/image/test-imagetools.sh index 6133340ba..144748945 100755 --- a/test/image/test-imagetools.sh +++ b/test/image/test-imagetools.sh @@ -204,18 +204,6 @@ main() list_image ${IMAGE_MULTI} assert_equal ${DUMPIMAGE_LIST} ${MKIMAGE_LIST} - # Compress and extract FIT images, compare the result - create_fit_image - extract_fit_image - for file in ${DATAFILES}; do - assert_equal ${file} ${SRCDIR}/${file} - done - assert_equal ${TEST_OUT} ${DATAFILE2} - - # List contents of FIT image and compares output from tools - list_image ${IMAGE_FIT_ITB} - assert_equal ${DUMPIMAGE_LIST} ${MKIMAGE_LIST} - # Remove files created cleanup -- 2.30.2