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>
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 7a34bfe..2842252 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -401,8 +401,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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);
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 3df7b52..e11cfbc 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -650,6 +650,8 @@
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 @@
/* 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 @@
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