From: Simon Glass <sjg@chromium.org>
Date: Tue, 16 Feb 2021 00:08:12 +0000 (-0700)
Subject: image: Check for unit addresses in FITs
X-Git-Url: http://git.dujemihanovic.xyz/img/sics.gif?a=commitdiff_plain;h=3f04db891a353f4b127ed57279279f851c6b4917;p=u-boot.git

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>
---

diff --git a/common/image-fit.c b/common/image-fit.c
index bcf395f6a1..28b3d2b191 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1568,6 +1568,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;
@@ -1589,10 +1617,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;
 		}
 	}
 
@@ -1955,10 +2000,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/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 22e8fc10d8..6dff6779d1 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -232,8 +232,8 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required,
         util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
         if full_test:
-            # Make sure that U-Boot checks that the config is in the list of hashed
-            # nodes. If it isn't, a security bypass is possible.
+            # Make sure that U-Boot checks that the config is in the list of
+            # hashed nodes. If it isn't, a security bypass is possible.
             ffit = '%stest.forged.fit' % tmpdir
             shutil.copyfile(fit, ffit)
             with open(ffit, 'rb') as fd:
@@ -263,10 +263,11 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required,
             shutil.copyfile(fit, efit)
             vboot_evil.add_evil_node(fit, efit, evil_kernel, 'kernel@')
 
+            msg = 'Signature checking prevents use of unit addresses (@) in nodes'
             util.run_and_log_expect_exception(
                 cons, [fit_check_sign, '-f', efit, '-k', dtb],
-                1, 'Node name contains @')
-            run_bootm(sha_algo, 'evil kernel@', 'Bad Data Hash', False, efit)
+                1, msg)
+            run_bootm(sha_algo, 'evil kernel@', msg, False, efit)
 
         # Create a new properly signed fit and replace header bytes
         make_fit('sign-configs-%s%s.its' % (sha_algo, padding))