fit: Don't allow verification of images with @ nodes

When searching for a node called 'fred', any unit address appended to the
name is ignored by libfdt, meaning that 'fred' can match 'fred@1'. This
means that we cannot be sure that the node originally intended is the one
that is used.

Disallow use of nodes with unit addresses.

Update the forge test also, since it uses @ addresses.

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>
Reviewed-By: Daniel Leidert <dleidert@debian.org>
Origin: https://github.com/u-boot/u-boot/commit/79af75f7776fc20b0d7eb6afe1e27c00fdb4b9b4
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-1.patch

diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index 5401d9411b986a34ae3328b1b25dc8def7c50310..7c19ac6dc214d33e11f74bc16788ef8a2adc0fbd 100644 (file)
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -162,6 +162,14 @@ static int fit_image_verify_sig(const void *fit, int image_noffset,
        fdt_for_each_subnode(noffset, fit, image_noffset) {
                const char *name = fit_get_name(fit, noffset, NULL);
 
+               /*
+                * We don't support this since libfdt considers names with the
+                * name root but different @ suffix to be equal
+                */
+               if (strchr(name, '@')) {
+                       err_msg = "Node name contains @";
+                       goto error;
+               }
                if (!strncmp(name, FIT_SIG_NODENAME,
                             strlen(FIT_SIG_NODENAME))) {
                        ret = fit_image_check_sig(fit, noffset, data,
@@ -411,9 +419,10 @@ error:
        return -EPERM;
 }
 
-int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
-                                   const void *sig_blob)
+static int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
+                                          const void *sig_blob)
 {
+       const char *name = fit_get_name(fit, conf_noffset, NULL);
        int noffset;
        int sig_node;
        int verified = 0;
@@ -421,6 +430,15 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset,
        bool reqd_policy_all = true;
        const char *reqd_mode;
 
+       /*
+        * We don't support this since libfdt considers names with the
+        * name root but different @ suffix to be equal
+        */
+       if (strchr(name, '@')) {
+               printf("Configuration node '%s' contains '@'\n", name);
+               return -EPERM;
+       }
+
        /* Work out what we need to verify */
        sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
        if (sig_node < 0) {

diff --git a/common/image-fit.c b/common/image-fit.c
index 6a8787ca0aa09c38689208b7dc74beabfc1c4c5b..abceae44e5e9e68888643d40a7b282048826dd0f 100644 (file)
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1353,21 +1353,31 @@ error:
  */
 int fit_image_verify(const void *fit, int image_noffset)
 {
+       const char *name = fit_get_name(fit, image_noffset, NULL);
        const void      *data;
        size_t          size;
-       int             noffset = 0;
        char            *err_msg = "";
 
+       if (strchr(name, '@')) {
+               /*
+                * We don't support this since libfdt considers names with the
+                * name root but different @ suffix to be equal
+                */
+               err_msg = "Node name contains @";
+               goto err;
+       }
        /* Get image data and data length */
        if (fit_image_get_data_and_size(fit, image_noffset, &data, &size)) {
                err_msg = "Can't get image data/size";
-               printf("error!\n%s for '%s' hash node in '%s' image node\n",
-                      err_msg, fit_get_name(fit, noffset, NULL),
-                      fit_get_name(fit, image_noffset, NULL));
-               return 0;
+               goto err;
        }
 
        return fit_image_verify_with_data(fit, image_noffset, data, size);
+
+err:
+       printf("error!\n%s in '%s' image node\n", err_msg,
+              fit_get_name(fit, image_noffset, NULL));
+       return 0;
 }
 
 /**

diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 84b3f9585057ed1f0a32af256c8f275bf1e6bdcf..6d5b43c3babb47b3157400db989efe925f274dc4 100755 (executable)
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -17,7 +17,7 @@ base_its = '''
         #address-cells = <1>;
 
         images {
-                kernel@1 {
+                kernel-1 {
                         data = /incbin/("%(kernel)s");
                         type = "kernel";
                         arch = "sandbox";
@@ -26,7 +26,7 @@ base_its = '''
                         load = <0x40000>;
                         entry = <0x8>;
                 };
-                kernel@2 {
+                kernel-2 {
                         data = /incbin/("%(loadables1)s");
                         type = "kernel";
                         arch = "sandbox";
@@ -35,19 +35,19 @@ base_its = '''
                         %(loadables1_load)s
                         entry = <0x0>;
                 };
-                fdt@1 {
+                fdt-1 {
                         description = "snow";
                         data = /incbin/("%(fdt)s");
                         type = "flat_dt";
                         arch = "sandbox";
                         %(fdt_load)s
                         compression = "%(compression)s";
-                        signature@1 {
+                        signature-1 {
                                 algo = "sha1,rsa2048";
                                 key-name-hint = "dev";
                         };
                 };
-                ramdisk@1 {
+                ramdisk-1 {
                         description = "snow";
                         data = /incbin/("%(ramdisk)s");
                         type = "ramdisk";
@@ -56,7 +56,7 @@ base_its = '''
                         %(ramdisk_load)s
                         compression = "%(compression)s";
                 };
-                ramdisk@2 {
+                ramdisk-2 {
                         description = "snow";
                         data = /incbin/("%(loadables2)s");
                         type = "ramdisk";
@@ -67,10 +67,10 @@ base_its = '''
                 };
         };
         configurations {
-                default = "conf@1";
-                conf@1 {
-                        kernel = "kernel@1";
-                        fdt = "fdt@1";
+                default = "conf-1";
+                conf-1 {
+                        kernel = "kernel-1";
+                        fdt = "fdt-1";
                         %(ramdisk_config)s
                         %(loadables_config)s
                 };
@@ -410,7 +410,7 @@ def test_fit(u_boot_console):
 
         # Try a ramdisk
         with cons.log.section('Kernel + FDT + Ramdisk load'):
-            params['ramdisk_config'] = 'ramdisk = "ramdisk@1";'
+            params['ramdisk_config'] = 'ramdisk = "ramdisk-1";'
             params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr']
             fit = make_fit(mkimage, params)
             cons.restart_uboot()
@@ -419,7 +419,7 @@ def test_fit(u_boot_console):
 
         # Configuration with some Loadables
         with cons.log.section('Kernel + FDT + Ramdisk load + Loadables'):
-            params['loadables_config'] = 'loadables = "kernel@2", "ramdisk@2";'
+            params['loadables_config'] = 'loadables = "kernel-2", "ramdisk-2";'
             params['loadables1_load'] = ('load = <%#x>;' %
                                          params['loadables1_addr'])
             params['loadables2_load'] = ('load = <%#x>;' %

diff --git a/test/py/tests/vboot_forge.py b/test/py/tests/vboot_forge.py
index 0fb7ef4024762498ff343717d648123f43636448..b41105bd0e3226ed013a6d80e480b0e8b10e2620 100644 (file)
--- a/test/py/tests/vboot_forge.py
+++ b/test/py/tests/vboot_forge.py
@@ -376,12 +376,12 @@ def manipulate(root, strblock):
     """
     Maliciously manipulates the structure to create a crafted FIT file
     """
-    # locate /images/kernel@1 (frankly, it just expects it to be the first one)
+    # locate /images/kernel-1 (frankly, it just expects it to be the first one)
     kernel_node = root[0][0]
     # clone it to save time filling all the properties
     fake_kernel = kernel_node.clone()
     # rename the node
-    fake_kernel.name = b'kernel@2'
+    fake_kernel.name = b'kernel-2'
     # get rid of signatures/hashes
     fake_kernel.children = []
     # NOTE: this simply replaces the first prop... either description or data
@@ -391,13 +391,13 @@ def manipulate(root, strblock):
     root[0].children.append(fake_kernel)
 
     # modify the default configuration
-    root[1].props[0].value = b'conf@2\x00'
+    root[1].props[0].value = b'conf-2\x00'
     # clone the first (only?) configuration
     fake_conf = root[1][0].clone()
     # rename and change kernel and fdt properties to select the crafted kernel
-    fake_conf.name = b'conf@2'
-    fake_conf.props[0].value = b'kernel@2\x00'
-    fake_conf.props[1].value = b'fdt@1\x00'
+    fake_conf.name = b'conf-2'
+    fake_conf.props[0].value = b'kernel-2\x00'
+    fake_conf.props[1].value = b'fdt-1\x00'
     # insert the new configuration under /configurations
     root[1].children.append(fake_conf)