Skip to content
Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
sjg20 authored and trini committed Feb 16, 2021
1 parent 8a7d4cf commit 79af75f
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 25 deletions.
22 changes: 20 additions & 2 deletions common/image-fit-sig.c
Expand Up @@ -149,6 +149,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,
Expand Down Expand Up @@ -398,16 +406,26 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset,
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;
int reqd_sigs = 0;
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) {
Expand Down
20 changes: 15 additions & 5 deletions common/image-fit.c
Expand Up @@ -1369,21 +1369,31 @@ int fit_image_verify_with_data(const void *fit, int image_noffset,
*/
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;
}

/**
Expand Down
24 changes: 12 additions & 12 deletions test/py/tests/test_fit.py
Expand Up @@ -17,7 +17,7 @@
#address-cells = <1>;
images {
kernel@1 {
kernel-1 {
data = /incbin/("%(kernel)s");
type = "kernel";
arch = "sandbox";
Expand All @@ -26,7 +26,7 @@
load = <0x40000>;
entry = <0x8>;
};
kernel@2 {
kernel-2 {
data = /incbin/("%(loadables1)s");
type = "kernel";
arch = "sandbox";
Expand All @@ -35,19 +35,19 @@
%(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";
Expand All @@ -56,7 +56,7 @@
%(ramdisk_load)s
compression = "%(compression)s";
};
ramdisk@2 {
ramdisk-2 {
description = "snow";
data = /incbin/("%(loadables2)s");
type = "ramdisk";
Expand All @@ -67,10 +67,10 @@
};
};
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
};
Expand Down Expand Up @@ -410,7 +410,7 @@ def run_fit_test(mkimage):

# 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()
Expand All @@ -419,7 +419,7 @@ def run_fit_test(mkimage):

# 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>;' %
Expand Down
12 changes: 6 additions & 6 deletions test/py/tests/vboot_forge.py
Expand Up @@ -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
Expand All @@ -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)

Expand Down

0 comments on commit 79af75f

Please sign in to comment.