From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 15 Feb 2021 17:08:12 -0700 Subject: [PATCH 8/8] image: Check for unit addresses in FITs In-Reply-To: <20210216000812.2091481-1-sjg@chromium.org> References: <20210216000812.2091481-1-sjg@chromium.org> Message-ID: <20210216000812.2091481-9-sjg@chromium.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 Reported-by: Bruce Monroe Reported-by: Arie Haenel Reported-by: Julien Lenoir --- common/image-fit.c | 56 ++++++++++++++++++++++++++++++++++--- test/py/tests/test_vboot.py | 9 +++--- 2 files changed, 57 insertions(+), 8 deletions(-) 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 @@ -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 22e8fc10d83..6dff6779d17 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)) -- 2.30.0.478.g8a0d178c01-goog