image-fit-sig: Validate hashed-strings region size

fit_config_check_sig() reads the hashed-strings property and uses
its size value without validation when building the region list for
signature verification. A crafted FIT image can specify an arbitrary
size, causing the hash calculation to read beyond the end of the FIT
image. The property length is also not checked, so a truncated
hashed-strings property causes strings[1] to be read past the end of
the property. This may result in the out-of-bounds read during signature
verification of an untrusted FIT.

Validate both the property length and that the declared strings region
fits within bounds before adding it to the region list.

Signed-off-by: Anton Ivanov <anton@binarly.io>

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 433df20281f55848edf1f350b7e39f0fd44d16b0..9b5ab75456171e2e45fb4dababc0f2d7139a7157 100644 (file)
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -452,6 +452,8 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
        int max_regions;
        char path[200];
        int count;
+       int len;
+       uint32_t size;
 
        debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, key_blob,
              fit_get_name(fit, noffset, NULL),
@@ -506,14 +508,27 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
        }
 
        /* Add the strings */
-       strings = fdt_getprop(fit, noffset, "hashed-strings", NULL);
+       strings = fdt_getprop(fit, noffset, "hashed-strings", &len);
        if (strings) {
+               if (len < (int)(2 * sizeof(fdt32_t))) {
+                       *err_msgp = "Invalid hashed-strings property";
+                       return -1;
+               }
+               size = fdt32_to_cpu(strings[1]);
+               /*
+                * The offset should be already validated by fdt_check_header();
+                * validate the size here.
+                */
+               if (size > fdt_size_dt_strings(fit)) {
+                       *err_msgp = "Strings region is out of bounds";
+                       return -1;
+               }
                /*
                 * The strings region offset must be a static 0x0.
                 * This is set in tool/image-host.c
                 */
                fdt_regions[count].offset = fdt_off_dt_strings(fit);
-               fdt_regions[count].size = fdt32_to_cpu(strings[1]);
+               fdt_regions[count].size = size;
                count++;
        }
 

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 55518bed07e9dfa0ecca6aaf5b10f08fd4f7e9f5..8fa8f2d59cfd4e151f7c3a670dabe25899445d1f 100644 (file)
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -415,6 +415,32 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required,
             ubman, [fit_check_sign, '-f', fit, '-k', dtb],
             1, 'Failed to verify required signature')
 
+        # Create a new properly signed fit and replace hashed-strings
+        # size property
+        make_fit('sign-configs-%s%s.its' % (sha_algo, padding), ubman, mkimage, dtc_args, datadir, fit)
+        sign_fit(sha_algo, sign_options)
+        utils.run_and_log(ubman, 'fdtput -t x %s %s hashed-strings 0' %
+                          (fit, sig_node))
+        run_bootm(sha_algo, 'Signed config with truncated hashed-strings',
+                  'Invalid hashed-strings property', False)
+        ubman.log.action('%s: Check truncated hashed-strings property' % sha_algo)
+
+        # size_dt_strings is at offset 32 in the FDT header
+        with open(fit, 'rb') as handle:
+            handle.seek(32)
+            size_dt_strings = struct.unpack(">I", handle.read(4))[0]
+        utils.run_and_log(ubman, 'fdtput -t x %s %s hashed-strings 0 %#x' %
+                          (fit, sig_node, size_dt_strings + 1))
+        run_bootm(sha_algo, 'Signed config with overflowed hashed-strings size',
+                  'Strings region is out of bounds', False)
+        ubman.log.action('%s: Check overflowed hashed-strings size' % sha_algo)
+
+        utils.run_and_log(ubman, 'fdtput -t x %s %s hashed-strings 0 %#x' %
+                          (fit, sig_node, size_dt_strings))
+        run_bootm(sha_algo, 'Signed config with in-bounds hashed-strings size',
+                  'Bad Data Hash', False)
+        ubman.log.action('%s: Check in-bounds hashed-strings size' % sha_algo)
+
     def test_required_key(sha_algo, padding, sign_options):
         """Test verified boot with the given hash algorithm.