Martin Roth merged this change.

View Change

util/cbfstool: Prevent overflow of 16 bit multiplications

Considering the following integer multiplication:

    u64 = u16 * u16

What on earth, one might wonder, is the problem with this? Well, due to
C's unfortunately abstruse integer semantics, both u16's are implicitly
converted to int before the multiplication, which cannot hold
all possible values of a u16 * u16. Even worse, after overflow the
intermediate result will be a negative number, which during the
conversion to a u64 will be sign-extended to a huge integer. Not good.

The solution is to manually cast one of the u16 to a u32 or u64, which
are large enough to not have any overflow and will prevent the implicit
conversion. The type of the u64 is preferred, though a u32 is used
instead of size_t, since that can change depending on the platform.

Change-Id: I5391221d46d620d0e5bd629e2f9680be7a53342e
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Found-by: Coverity CID 12297{03,04,05,06,07,08,09,10}
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33986
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
M util/cbfstool/elfheaders.c
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/util/cbfstool/elfheaders.c b/util/cbfstool/elfheaders.c
index 8da54d0..676a635 100644
--- a/util/cbfstool/elfheaders.c
+++ b/util/cbfstool/elfheaders.c
@@ -262,7 +262,8 @@
 	 * per the ELF spec, You'd be surprised how many ELF
 	 * readers miss this little detail.
 	 */
-	buffer_splice(&b, in, ehdr->e_phoff, ehdr->e_phentsize * ehdr->e_phnum);
+	buffer_splice(&b, in, ehdr->e_phoff,
+		      (uint32_t)ehdr->e_phentsize * ehdr->e_phnum);
 	if (check_size(in, ehdr->e_phoff, buffer_size(&b), "program headers"))
 		return -1;
 
@@ -304,7 +305,8 @@
 	 * per the ELF spec, You'd be surprised how many ELF
 	 * readers miss this little detail.
 	 */
-	buffer_splice(&b, in, ehdr->e_shoff, ehdr->e_shentsize * ehdr->e_shnum);
+	buffer_splice(&b, in, ehdr->e_shoff,
+		      (uint32_t)ehdr->e_shentsize * ehdr->e_shnum);
 	if (check_size(in, ehdr->e_shoff, buffer_size(&b), "section headers"))
 		return -1;
 
@@ -1180,8 +1182,8 @@
 	ew->ehdr.e_shnum = ew->num_secs;
 	metadata_size = 0;
 	metadata_size += ew->ehdr.e_ehsize;
-	metadata_size += ew->ehdr.e_shnum * ew->ehdr.e_shentsize;
-	metadata_size += ew->ehdr.e_phnum * ew->ehdr.e_phentsize;
+	metadata_size += (Elf64_Xword)ew->ehdr.e_shnum * ew->ehdr.e_shentsize;
+	metadata_size += (Elf64_Xword)ew->ehdr.e_phnum * ew->ehdr.e_phentsize;
 	shstroffset = metadata_size;
 	/* Align up section header string size and metadata size to 4KiB */
 	metadata_size = ALIGN(metadata_size + shstrlen, 4096);
@@ -1200,11 +1202,11 @@
 	 */
 	ew->ehdr.e_shoff = ew->ehdr.e_ehsize;
 	ew->ehdr.e_phoff = ew->ehdr.e_shoff +
-	                   ew->ehdr.e_shnum * ew->ehdr.e_shentsize;
+			   (Elf64_Off)ew->ehdr.e_shnum * ew->ehdr.e_shentsize;
 
 	buffer_splice(&metadata, out, 0, metadata_size);
 	buffer_splice(&phdrs, out, ew->ehdr.e_phoff,
-	              ew->ehdr.e_phnum * ew->ehdr.e_phentsize);
+		      (uint32_t)ew->ehdr.e_phnum * ew->ehdr.e_phentsize);
 	buffer_splice(&data, out, metadata_size, program_size);
 	/* Set up the section header string table contents. */
 	strtab = &ew->shstrtab_sec->content;

To view, visit change 33986. To unsubscribe, or for help writing mail filters, visit settings.