fit_image_get_data() uses the data-position, data-offset, and data-size FIT properties without bounds checking. A crafted FIT image can specify values that cause out-of-bounds read during signature verification of an untrusted FIT.
Validate that the external data offset and size are non-negative, and that the data region fits within the FIT image bounds. Signed-off-by: Anton Ivanov <[email protected]> --- Changes in v4: - Add test for external data bounds validation - Clarify vulnerability reachability in the commit message Changes in v3: - Update From and Signed-off-by to personal email Changes in v2: - Rewrite commit message to be concise per maintainer feedback boot/image-fit.c | 16 ++++ test/py/tests/test_vboot.py | 165 ++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+) diff --git a/boot/image-fit.c b/boot/image-fit.c index b0fcaf6e17f..b657142f626 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -1084,8 +1084,24 @@ int fit_image_get_data(const void *fit, int noffset, const void **data, if (external_data) { debug("External Data\n"); + if (offset < 0 || offset > UINTPTR_MAX - (uintptr_t)fit) { + printf("Invalid external data offset: %d\n", offset); + return -1; + } + ret = fit_image_get_data_size(fit, noffset, &len); if (!ret) { + if (len < 0) { + printf("Invalid external data size: %d\n", len); + return -1; + } +#if CONFIG_IS_ENABLED(FIT_SIGNATURE) + if (len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset) { + printf("FIT external data is out of bounds (offset=%d, size=%d)\n", + offset, len); + return -1; + } +#endif *data = fit + offset; *size = len; } diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 55518bed07e..85a1bd56af6 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -563,6 +563,171 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required, ubman.restart_uboot() [email protected]('sandbox') [email protected]('fit_signature') [email protected]('dtc') [email protected]('fdtput') [email protected]('openssl') +def test_vboot_ext_data_bounds(ubman): + """Test that malformed external-data properties are rejected. + + A signed FIT with external data exposes 'data-position', 'data-offset' and + 'data-size' properties. U-Boot must validate these before hashing the image + components, otherwise a crafted FIT could trigger an out-of-bounds access + during signature verification. + + These checks are independent of the hashing algorithm, so a single signing + configuration is enough. + + This works using sandbox only as it needs to update the device tree used + by U-Boot to hold public keys from the signing process. + """ + sha_algo = 'sha256' + + def run_bootm(test_type, expect_string): + """Run a 'bootm' command in U-Boot and expect it to fail. + + This always starts a fresh U-Boot instance since the device tree may + contain a new public key. + + Args: + test_type: A string identifying the test type. + expect_string: A string which is expected in the output. + """ + ubman.restart_uboot() + with ubman.log.section('Verified boot %s %s' % (sha_algo, test_type)): + output = ubman.run_command_list( + ['host load hostfs - 100 %s' % fit, + 'fdt addr 100', + 'bootm 100']) + assert expect_string in ''.join(output) + assert 'sandbox: continuing, as we cannot run' not in ''.join(output) + + def sign_fit(options): + """Sign the FIT + + Signs the FIT and writes the signature into it. It also writes the + public key into the dtb. + + Args: + options: Options to provide to mkimage. + """ + args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit] + if options: + args += options.split(' ') + ubman.log.action('%s: Sign images' % sha_algo) + utils.run_and_log(ubman, args) + + def create_rsa_pair(name): + """Generate a new RSA key pair and certificate. + + Args: + name: Name of the key (e.g. 'dev') + """ + public_exponent = 65537 + utils.run_and_log(ubman, 'openssl genpkey -algorithm RSA -out %s%s.key ' + '-pkeyopt rsa_keygen_bits:2048 ' + '-pkeyopt rsa_keygen_pubexp:%d' % + (tmpdir, name, public_exponent)) + + # Create a certificate containing the public key + utils.run_and_log(ubman, 'openssl req -batch -new -x509 -key %s%s.key ' + '-out %s%s.crt' % (tmpdir, name, tmpdir, name)) + + def set_external_data(prop, value): + """Set an external-data property of the kernel image. + + Args: + prop: Property name + value: The new value of the property + """ + utils.run_and_log( + ubman, 'fdtput -t x %s /images/kernel %s %#x' % (fit, prop, value) + ) + + def make_signed_fit(): + """Build a fresh signed FIT with external data. + + sign_fit() overwrites the FIT, so a new one is built before each test + case mutates its external-data properties. + """ + make_fit('sign-configs-%s.its' % sha_algo, ubman, mkimage, dtc_args, + datadir, fit) + sign_fit('-E') + + tmpdir = os.path.join(ubman.config.result_dir, 'ext-data-bounds') + '/' + if not os.path.exists(tmpdir): + os.mkdir(tmpdir) + datadir = ubman.config.source_dir + '/test/py/tests/vboot/' + fit = '%stest.fit' % tmpdir + mkimage = ubman.config.build_dir + '/tools/mkimage' + dtc_args = '-I dts -O dtb -i %s' % tmpdir + dtb = '%ssandbox-u-boot.dtb' % tmpdir + + bcfg = ubman.config.buildconfig + max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0) + + create_rsa_pair('dev') + + # Create a kernel image filled with zeroes + with open('%stest-kernel.bin' % tmpdir, 'wb') as fd: + fd.write(500 * b'\0') + + testcases = [ + ('negative data-position', + {'data-position': 0xFFFFFFFF}, 'Invalid external data offset'), + ('negative data-offset', + {'data-offset': 0x80000000}, 'Invalid external data offset'), + ('negative data-size', + {'data-size': 0xFFFFFFFF}, 'Invalid external data size'), + ('off-bounds data-position', + {'data-position': 0x7FFFFFFF}, 'FIT external data is out of bounds'), + ('off-bounds data-offset', + {'data-offset': 0x10000000}, 'FIT external data is out of bounds'), + ('oversized data-size', + {'data-size': 0x7FFFFFFF}, 'FIT external data is out of bounds'), + ('off-bounds data-position', + {'data-position': max_size + 1, 'data-size': 0}, + 'FIT external data is out of bounds'), + ('off-bounds data-offset', + {'data-offset': max_size + 1, 'data-size': 0}, + 'FIT external data is out of bounds'), + ('oversized data-size', + {'data-position': 0x0, 'data-size': max_size + 1}, + 'FIT external data is out of bounds'), + ('in-bounds data-position', + {'data-position': max_size, 'data-size': 0}, 'Bad Data Hash'), + ('in-bounds data-offset', + {'data-offset': max_size, 'data-size': 0}, 'Bad Data Hash'), + ('in-bounds data-size', + {'data-position': 0x0, 'data-size': max_size}, 'Bad Data Hash'), + ] + + # We need to use our own device tree file. Remember to restore it + # afterwards. + old_dtb = ubman.config.dtb + try: + ubman.config.dtb = dtb + + # Compile our device tree files for kernel and U-Boot. These are + # regenerated here since mkimage will modify them (by adding a + # public key) below. + dtc('sandbox-kernel.dts', ubman, dtc_args, datadir, tmpdir, dtb) + dtc('sandbox-u-boot.dts', ubman, dtc_args, datadir, tmpdir, dtb) + + ubman.log.action( + '%s: Test signed FIT with malformed external-data properties' % sha_algo) + for desc, props, expect_string in testcases: + make_signed_fit() + for prop, value in props.items(): + set_external_data(prop, value) + run_bootm('Signed config with %s' % desc, expect_string) + finally: + # Go back to the original U-Boot with the correct dtb. + ubman.config.dtb = old_dtb + ubman.restart_uboot() + + TESTDATA_IN = [ ['sha1-basic', 'sha1', '', None, False], ['sha1-pad', 'sha1', '', '-E -p 0x10000', False], -- 2.53.0
