diff options
author | Jeremy Compostella <jeremy.compostella@intel.com> | 2023-10-23 13:00:33 -0700 |
---|---|---|
committer | Matt DeVillier <matt.devillier@amd.corp-partner.google.com> | 2023-10-29 14:23:23 +0000 |
commit | 66df100930d9259238a402db3fe368b65647a41b (patch) | |
tree | 3e54a4efce40b63361640859369fc2c5ff0e536a /util/cbfstool/cbfs_image.c | |
parent | 3e57c574803deedb49b7cc330f7445c5244e097b (diff) |
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>
Diffstat (limited to 'util/cbfstool/cbfs_image.c')
-rw-r--r-- | util/cbfstool/cbfs_image.c | 36 |
1 files changed, 24 insertions, 12 deletions
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 7a34bfe58f..2842252585 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -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); |