cbfstool: Fix CBFS header buffer overflow

In the unlikely but possible event where the name of the CBFS file is
longer than 232 characters, `cbfs_create_file_header()' would overflow
the buffer it allocated when it copies the CBFS filename.

Change-Id: If1825b5af21f7a20ce2a7ccb2d45b195c2fb67b0
Signed-off-by: Jeremy Compostella <jeremy.compostella@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78500
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Eric Lai <ericllai@google.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
This commit is contained in:
Jeremy Compostella 2023-10-23 13:00:33 -07:00 committed by Matt DeVillier
parent 3e57c57480
commit 66df100930
2 changed files with 30 additions and 12 deletions

View File

@ -401,8 +401,8 @@ int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst)
if (last_entry_size < 0)
WARN("No room to create the last entry!\n");
else
cbfs_create_empty_entry(dst_entry, CBFS_TYPE_NULL,
last_entry_size, "");
return cbfs_create_empty_entry(dst_entry, CBFS_TYPE_NULL,
last_entry_size, "");
return 0;
}
@ -437,8 +437,10 @@ int cbfs_expand_to_region(struct buffer *region)
cbfs_calculate_file_header_size("") - sizeof(int32_t);
if (last_entry_size > 0) {
cbfs_create_empty_entry(entry, CBFS_TYPE_NULL,
last_entry_size, "");
if (cbfs_create_empty_entry(entry, CBFS_TYPE_NULL,
last_entry_size, ""))
return 1;
/* If the last entry was an empty file, merge them. */
cbfs_legacy_walk(&image, cbfs_merge_empty_entry, NULL);
}
@ -586,8 +588,9 @@ int cbfs_compact_instance(struct cbfs_image *image)
prev_size -= spill_size + empty_metadata_size;
/* Create new empty file. */
cbfs_create_empty_entry(cur, CBFS_TYPE_NULL,
prev_size, "");
if (cbfs_create_empty_entry(cur, CBFS_TYPE_NULL,
prev_size, ""))
return 1;
/* Merge any potential empty entries together. */
cbfs_legacy_walk(image, cbfs_merge_empty_entry, NULL);
@ -641,7 +644,8 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
if (header_offset - addr > min_entry_size) {
DEBUG("|min|...|header|content|... <create new entry>\n");
len = header_offset - addr - min_entry_size;
cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, "");
if (cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, ""))
return -1;
if (verbose > 1) cbfs_print_entry_info(image, entry, stderr);
entry = cbfs_find_next_entry(image, entry);
addr = cbfs_get_entry_addr(image, entry);
@ -713,7 +717,8 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
buffer_size(&image->buffer) - sizeof(int32_t)) {
len -= sizeof(int32_t);
}
cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, "");
if (cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, ""))
return -1;
if (verbose > 1) cbfs_print_entry_info(image, entry, stderr);
return 0;
}
@ -1650,9 +1655,7 @@ int cbfs_merge_empty_entry(struct cbfs_image *image, struct cbfs_file *entry,
uint32_t addr = cbfs_get_entry_addr(image, entry);
size_t len = next_addr - addr - cbfs_calculate_file_header_size("");
DEBUG("join_empty_entry: [0x%x, 0x%x) len=%zu\n", addr, next_addr, len);
cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, "");
return 0;
return cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, "");
}
int cbfs_legacy_walk(struct cbfs_image *image, cbfs_entry_callback callback,
@ -1785,13 +1788,19 @@ int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry)
struct cbfs_file *cbfs_create_file_header(int type,
size_t len, const char *name)
{
size_t header_size = cbfs_calculate_file_header_size(name);
if (header_size > CBFS_METADATA_MAX_SIZE) {
ERROR("'%s' name too long to fit in CBFS header\n", name);
return NULL;
}
struct cbfs_file *entry = malloc(CBFS_METADATA_MAX_SIZE);
memset(entry, CBFS_CONTENT_DEFAULT_VALUE, CBFS_METADATA_MAX_SIZE);
memcpy(entry->magic, CBFS_FILE_MAGIC, sizeof(entry->magic));
entry->type = htobe32(type);
entry->len = htobe32(len);
entry->attributes_offset = 0;
entry->offset = htobe32(cbfs_calculate_file_header_size(name));
entry->offset = htobe32(header_size);
memset(entry->filename, 0, be32toh(entry->offset) - sizeof(*entry));
strcpy(entry->filename, name);
return entry;
@ -1801,6 +1810,9 @@ int cbfs_create_empty_entry(struct cbfs_file *entry, int type,
size_t len, const char *name)
{
struct cbfs_file *tmp = cbfs_create_file_header(type, len, name);
if (!tmp)
return -1;
memcpy(entry, tmp, be32toh(tmp->offset));
free(tmp);
memset(CBFS_SUBHEADER(entry), CBFS_CONTENT_DEFAULT_VALUE, len);

View File

@ -650,6 +650,8 @@ static int cbfs_add_integer_component(const char *name,
header = cbfs_create_file_header(CBFS_TYPE_RAW,
buffer.size, name);
if (!header)
goto done;
enum vb2_hash_algorithm algo = get_mh_cache()->cbfs_hash.algo;
if (algo != VB2_HASH_INVALID)
@ -774,6 +776,8 @@ static int cbfs_add_master_header(void)
/* Never add a hash attribute to the master header. */
header = cbfs_create_file_header(CBFS_TYPE_CBFSHEADER,
buffer_size(&buffer), name);
if (!header)
goto done;
if (cbfs_add_entry(&image, &buffer, 0, header, 0) != 0) {
ERROR("Failed to add cbfs master header into ROM image.\n");
goto done;
@ -915,6 +919,8 @@ static int cbfs_add_component(const char *filename,
struct cbfs_file *header =
cbfs_create_file_header(param.type, buffer.size, name);
if (!header)
goto error;
/* Bootblock and CBFS header should never have file hashes. When adding
the bootblock it is important that we *don't* look up the metadata