mirror of https://github.com/wolfSSL/wolfBoot.git
F-9757: length-bound the FIT name/compression string properties
fit_find_images() took the FIT configuration's image names (kernel/fdt/ramdisk/fpga) and the configuration name (default) straight from fdt_getprop() and passed them on to fdt_find_node_offset(), which strlen()s them; fit_load_image_inner() strcmp()'d the compression property after only checking it was non-empty. A property value not NUL-terminated within its declared length makes those calls scan past the property - and past the end of the blob for a property at the tail. Add fit_getprop_string(), which returns the property value only when it is NUL-terminated within its declared length, and use it for the five name properties (a malformed value is rejected and the type-based search still applies). Compare compression within the declared length: the value must be exactly "gzip" or "none"; any other shape fails closed with the existing unsupported-compression path instead of being strcmp()'d past the property. unit-fdt gains a FIT whose configuration kernel property is unterminated (the valid default is still honored, the image name is rejected); unit-fit-gzip gains a truncated compression="none" value, which used to pass the subimage through as raw and now fails closed. Both build variants (gzip enabled/disabled) run it.pull/868/head
parent
19d8a9e346
commit
fe1dcb080d
44
src/fdt.c
44
src/fdt.c
|
|
@ -940,6 +940,22 @@ int fdt_fixup_val64(void* fdt, int off, const char* node, const char* name,
|
|||
|
||||
|
||||
/* FIT Specific */
|
||||
|
||||
/* Returns the property value only when it is a NUL-terminated C string
|
||||
* within its declared length, else NULL: property values are opaque
|
||||
* byte arrays and the names taken from them are passed to
|
||||
* fdt_find_node_offset()/strcmp(), which strlen() them. */
|
||||
static const char* fit_getprop_string(const void* fdt, int offset,
|
||||
const char* name)
|
||||
{
|
||||
int len = 0;
|
||||
const char* val = (const char*)fdt_getprop(fdt, offset, name, &len);
|
||||
|
||||
if (val == NULL || len <= 0 || memchr(val, '\0', len) == NULL)
|
||||
return NULL;
|
||||
return val;
|
||||
}
|
||||
|
||||
const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_dt,
|
||||
const char** pramdisk, const char** pfpga)
|
||||
{
|
||||
|
|
@ -968,19 +984,16 @@ const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_
|
|||
if (conf == NULL)
|
||||
#endif
|
||||
{
|
||||
val = fdt_getprop(fdt, off, "default", &len);
|
||||
if (val != NULL && len > 0) {
|
||||
conf = (const char*)val;
|
||||
}
|
||||
conf = fit_getprop_string(fdt, off, "default");
|
||||
}
|
||||
}
|
||||
if (conf != NULL) {
|
||||
off = fdt_find_node_offset(fdt, -1, conf);
|
||||
if (off > 0) {
|
||||
kernel = fdt_getprop(fdt, off, "kernel", &len);
|
||||
flat_dt = fdt_getprop(fdt, off, "fdt", &len);
|
||||
ramdisk = fdt_getprop(fdt, off, "ramdisk", &len);
|
||||
fpga = fdt_getprop(fdt, off, "fpga", &len);
|
||||
kernel = fit_getprop_string(fdt, off, "kernel");
|
||||
flat_dt = fit_getprop_string(fdt, off, "fdt");
|
||||
ramdisk = fit_getprop_string(fdt, off, "ramdisk");
|
||||
fpga = fit_getprop_string(fdt, off, "fpga");
|
||||
}
|
||||
}
|
||||
if (kernel == NULL) {
|
||||
|
|
@ -1219,11 +1232,20 @@ static void* fit_load_image_inner(void* fdt, const char* image, int* lenp,
|
|||
* raw. */
|
||||
comp = (const char*)fdt_getprop(fdt, off, "compression",
|
||||
&complen);
|
||||
if (comp != NULL && complen > 0) {
|
||||
if (strcmp(comp, "gzip") == 0) {
|
||||
/* Compare within the declared property length: the value
|
||||
* must be exactly "gzip" or "none" (NUL-terminated). Any
|
||||
* other shape - including an unterminated value - fails
|
||||
* closed instead of being strncmp()'d past the property. */
|
||||
if (comp != NULL) {
|
||||
if (complen == 5 && comp[4] == '\0' &&
|
||||
memcmp(comp, "gzip", 4) == 0) {
|
||||
is_gzip = 1;
|
||||
}
|
||||
else if (strcmp(comp, "none") != 0) {
|
||||
else if (complen == 5 && comp[4] == '\0' &&
|
||||
memcmp(comp, "none", 4) == 0) {
|
||||
/* uncompressed */
|
||||
}
|
||||
else {
|
||||
is_unknown_comp = 1;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -341,6 +341,72 @@ START_TEST(test_fdt_check_header_rejects_overlapping_areas)
|
|||
}
|
||||
END_TEST
|
||||
|
||||
/* FIT whose configuration `kernel` property is not NUL-terminated
|
||||
* within its declared length: fit_find_images() must still honor the
|
||||
* valid `default` but reject the malformed image name instead of
|
||||
* passing it on as a C string. */
|
||||
static const uint8_t fit_cfg_unterminated_kernel[] = {
|
||||
/* header */
|
||||
0xd0, 0x0d, 0xfe, 0xed, /* magic */
|
||||
0x00, 0x00, 0x00, 0xa7, /* totalsize = 167 */
|
||||
0x00, 0x00, 0x00, 0x38, /* off_dt_struct = 56 */
|
||||
0x00, 0x00, 0x00, 0x98, /* off_dt_strings = 152 */
|
||||
0x00, 0x00, 0x00, 0x28, /* off_mem_rsvmap = 40 */
|
||||
0x00, 0x00, 0x00, 0x11, /* version = 17 */
|
||||
0x00, 0x00, 0x00, 0x10, /* last_comp_version = 16 */
|
||||
0x00, 0x00, 0x00, 0x00, /* boot_cpuid_phys */
|
||||
0x00, 0x00, 0x00, 0x0f, /* size_dt_strings = 15 */
|
||||
0x00, 0x00, 0x00, 0x60, /* size_dt_struct = 96 */
|
||||
/* mem_rsvmap terminator (offset 40) */
|
||||
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
||||
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
||||
/* struct block (offset 56) */
|
||||
0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE root */
|
||||
0x00, 0x00, 0x00, 0x00, /* "" */
|
||||
0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE */
|
||||
0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x75, 0x72,
|
||||
0x61, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x00, 0x00, /* "configurations\0" */
|
||||
0x00, 0x00, 0x00, 0x03, /* FDT_PROP */
|
||||
0x00, 0x00, 0x00, 0x07, /* len = 7 */
|
||||
0x00, 0x00, 0x00, 0x00, /* nameoff = 0 ("default") */
|
||||
0x63, 0x6f, 0x6e, 0x66, 0x2d, 0x31, 0x00, 0x00, /* "conf-1\0" */
|
||||
0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE */
|
||||
0x63, 0x6f, 0x6e, 0x66, 0x2d, 0x31, 0x00, 0x00, /* "conf-1\0" */
|
||||
0x00, 0x00, 0x00, 0x03, /* FDT_PROP */
|
||||
0x00, 0x00, 0x00, 0x08, /* len = 8 */
|
||||
0x00, 0x00, 0x00, 0x08, /* nameoff = 8 ("kernel") */
|
||||
0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x2d, 0x31, /* "kernel-1" -- no NUL */
|
||||
0x00, 0x00, 0x00, 0x02, /* END_NODE conf-1 */
|
||||
0x00, 0x00, 0x00, 0x02, /* END_NODE configurations */
|
||||
0x00, 0x00, 0x00, 0x02, /* END_NODE root */
|
||||
0x00, 0x00, 0x00, 0x09, /* FDT_END */
|
||||
/* strings block (offset 152) */
|
||||
0x64, 0x65, 0x66, 0x61, 0x75, 0x6c, 0x74, 0x00, /* "default\0" */
|
||||
0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x00, /* "kernel\0" */
|
||||
};
|
||||
|
||||
START_TEST(test_fit_find_images_rejects_unterminated_image_name)
|
||||
{
|
||||
static uint8_t fit_scratch[sizeof(fit_cfg_unterminated_kernel)];
|
||||
const char *conf = NULL, *kern = NULL, *fdt = NULL;
|
||||
const char *rd = NULL, *fpga = NULL;
|
||||
|
||||
memcpy(fit_scratch, fit_cfg_unterminated_kernel, sizeof(fit_scratch));
|
||||
|
||||
conf = fit_find_images(fit_scratch, &kern, &fdt, &rd, &fpga);
|
||||
|
||||
/* The valid `default` is still honored... */
|
||||
ck_assert_str_eq(conf, "conf-1");
|
||||
/* ...but the config's `kernel` property is not NUL-terminated
|
||||
* within its declared length, so it must be rejected rather than
|
||||
* passed on to fdt_find_node_offset()/strlen(). */
|
||||
ck_assert_ptr_null(kern);
|
||||
ck_assert_ptr_null(fdt);
|
||||
ck_assert_ptr_null(rd);
|
||||
ck_assert_ptr_null(fpga);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
static Suite *fdt_suite(void)
|
||||
{
|
||||
Suite *s = suite_create("fdt");
|
||||
|
|
@ -359,6 +425,7 @@ static Suite *fdt_suite(void)
|
|||
tcase_add_test(tc, test_fdt_compatible_prefix_entry_no_match);
|
||||
tcase_add_test(tc, test_fdt_check_header_rejects_unbounded_string_area);
|
||||
tcase_add_test(tc, test_fdt_check_header_rejects_overlapping_areas);
|
||||
tcase_add_test(tc, test_fit_find_images_rejects_unterminated_image_name);
|
||||
suite_add_tcase(s, tc);
|
||||
|
||||
tcase_set_timeout(tc_dos, 5);
|
||||
|
|
|
|||
|
|
@ -269,6 +269,44 @@ END_TEST
|
|||
|
||||
#endif /* WOLFBOOT_GZIP */
|
||||
|
||||
START_TEST(test_fit_to_none_unterminated_fails_closed)
|
||||
{
|
||||
/* The `compression` property is truncated to 4 bytes ("none" with
|
||||
* no NUL): the declared length no longer covers the terminator.
|
||||
* Before the fix the value was strcmp()'d and the following byte
|
||||
* (the dropped NUL) terminated it, so the subimage was passed
|
||||
* through as raw; a malformed value must fail closed. */
|
||||
uint8_t buf[64];
|
||||
int len = -1;
|
||||
void *ret;
|
||||
static uint8_t fit_scratch[sizeof(fit_with_none_comp)];
|
||||
uint8_t *p;
|
||||
unsigned i;
|
||||
|
||||
memcpy(fit_scratch, fit_with_none_comp, sizeof(fit_scratch));
|
||||
|
||||
/* the "none" value appears once in the blob (not as a C string -
|
||||
* scan raw bytes); the property length word sits 8 bytes before
|
||||
* the data */
|
||||
p = NULL;
|
||||
for (i = 0; i + 4 <= sizeof(fit_scratch); i++) {
|
||||
if (memcmp(fit_scratch + i, "none", 4) == 0) {
|
||||
p = fit_scratch + i;
|
||||
break;
|
||||
}
|
||||
}
|
||||
ck_assert_ptr_nonnull(p);
|
||||
p[-8] = 0;
|
||||
p[-7] = 0;
|
||||
p[-6] = 0;
|
||||
p[-5] = 4;
|
||||
|
||||
ret = fit_load_image_to(fit_scratch, "kernel-1",
|
||||
buf, (uint32_t)sizeof(buf), &len);
|
||||
ck_assert_ptr_null(ret);
|
||||
}
|
||||
END_TEST
|
||||
|
||||
START_TEST(test_fit_to_lzma_unknown_returns_null)
|
||||
{
|
||||
/* Independent of WOLFBOOT_GZIP - any unknown compression scheme is
|
||||
|
|
@ -320,6 +358,7 @@ static Suite *fit_gzip_suite(void)
|
|||
tcase_add_test(tc, test_fit_to_gzip_disabled_returns_null);
|
||||
#endif
|
||||
tcase_add_test(tc, test_fit_to_lzma_unknown_returns_null);
|
||||
tcase_add_test(tc, test_fit_to_none_unterminated_fails_closed);
|
||||
tcase_add_test(tc, test_fit_to_none_oversized_rejected);
|
||||
|
||||
suite_add_tcase(s, tc);
|
||||
|
|
|
|||
Loading…
Reference in New Issue