LM
/
u-boot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Simon Glass 2021-02-15 17:08:12 -07:00 committed by Tom Rini
parent 124c255731
commit 3f04db891a
2 changed files with 57 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
common/image-fit.c
View File

@ -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 -EINVAL;
return ret;
}
}
if (ret) {
log_debug("FIT check error %d\n", ret);
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) {

9
test/py/tests/test_vboot.py
View File

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