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>

2 files changed, 30 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);
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 3df7b52089..e11cfbc126 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -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