[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 <sjg@chromium.org>
Reported-by: Bruce Monroe <bruce.monroe@intel.com>
Reported-by: Arie Haenel <arie.haenel@intel.com>
Reported-by: Julien Lenoir <julien.lenoir@intel.com>
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 <dleidert@debian.org>
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

diff --git a/common/image-fit.c b/common/image-fit.c
index b9edcc7a4b2d579f510f060e3f40497145f387b5..1b475224ee94155abd86e133a3c4d8f56b8193a2 100644 (file)
--- 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 6133340ba3a980a9db1c8bf479975b1bea4fabc1..144748945c511c8b4a64e824da6e3e1cc499077b 100755 (executable)
--- 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