CVE-2021-27138: Insufficient Information (NVD-CWE-noinfo)
value: The boot loader in Das U-Boot before 2021.04-rc2 mishandles use of unit addresses in a FIT.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 7.8
Published: 2021-02-17T23:15Z
Repository: https://github.com/u-boot/u-boot
Language: C
Files changed: common/image-fit.c, test/py/tests/test_vboot.py
Commit date: 2021-02-15 22:31:54 -0500
Commit message: 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: diff --git a/common/image-fit.c b/common/image-fit.c
index bcf395f6a18..28b3d2b1911 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1566,10 +1566,38 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp)
 	if (fit_image_get_comp(fit, noffset, &image_comp))
 		return 0;
 	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;
 
 	/* A FIT image must be a valid FDT */
@@ -1587,14 +1615,31 @@ int fit_check_format(const void *fit, ulong size)
 		 * control this.
 		 */
 		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;
 		}
 	}
 
 	/* mandatory / node 'description' property */
 	if (!fdt_getprop(fit, 0, FIT_DESC_PROP, NULL)) {
@@ -1953,14 +1998,17 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	fit_base_uname_config = NULL;
 	prop_name = fit_get_image_type_property(image_type);
 	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) {
 		/* get FIT component image node offset */
 		bootstage_mark(bootstage_id + BOOTSTAGE_SUB_UNIT_NAME);
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 22e8fc10d83..6dff6779d17 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -230,12 +230,12 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required,
         cons.log.action('%s: Check signed config on the host' % sha_algo)
 
         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:
                 root, strblock = vboot_forge.read_fdt(fd)
             root, strblock = vboot_forge.manipulate(root, strblock)
@@ -261,14 +261,15 @@ def test_vboot(u_boot_console, sha_algo, padding, sign_options, required,
             # Try adding an @ to the kernel node name. This should be detected.
             efit = '%stest.evilk.fit' % tmpdir
             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))
         sign_fit(sha_algo, sign_options)
         bcfg = u_boot_console.config.buildconfig

Vulnerable code: 			return -EINVAL;
	if (fit_check_format(fit, IMAGE_SIZE_INVAL)) {
		printf("Bad FIT %s image format!\n", prop_name);
		return -ENOEXEC;
            # 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.
                1, 'Node name contains @')
            run_bootm(sha_algo, 'evil kernel@', 'Bad Data Hash', False, efit)
Fixed code: /**
 * 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;
}

		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;
			}
		}
			return ret;
	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");
		return ret;
            # 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.
            msg = 'Signature checking prevents use of unit addresses (@) in nodes'
                1, msg)
            run_bootm(sha_algo, 'evil kernel@', msg, False, efit)