Boot: Add overflow checks to TLV iterator
Add overflow checks to TLV iterator function. Some of the
overflow checks were lost from certain parts of the code when
the TLV iterator functions were introduced. Use overflow-safe
addition functions instead of the overflow checking functions
and remove these as they were overlapping in functionality.
Change-Id: I3b08ac5f84774cb17150a3c5d1c7a9781241fda6
Signed-off-by: David Vincze <david.vincze@arm.com>
diff --git a/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c b/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
index 4cddec3..a546916 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
@@ -696,33 +696,3 @@
flash_area_close(fap);
return rc;
}
-
-/**
- * Checks whether on overflow can happen during a summation operation
- *
- * @param a First operand of summation
- *
- * @param b Second operand of summation
- *
- * @return True in case of overflow, false otherwise
- */
-bool
-boot_add_uint32_overflow_check(uint32_t a, uint32_t b)
-{
- return (a > UINT32_MAX - b);
-}
-
-/**
- * Checks whether on overflow can happen during a summation operation
- *
- * @param a First operand of summation
- *
- * @param b Second operand of summation
- *
- * @return True in case of overflow, false otherwise
- */
-bool
-boot_add_uint16_overflow_check(uint16_t a, uint16_t b)
-{
- return (a > UINT16_MAX - b);
-}
diff --git a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
index f2965e5..a603b01 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
@@ -233,8 +233,6 @@
uint8_t image_num);
int boot_write_swap_size(const struct flash_area *fap, uint32_t swap_size);
int boot_read_swap_size(int image_index, uint32_t *swap_size);
-bool boot_add_uint32_overflow_check(uint32_t a, uint32_t b);
-bool boot_add_uint16_overflow_check(uint16_t a, uint16_t b);
/**
* Safe (non-overflowing) uint32_t addition. Returns true, and stores
diff --git a/bl2/ext/mcuboot/bootutil/src/loader.c b/bl2/ext/mcuboot/bootutil/src/loader.c
index 9387174..4a08b24 100644
--- a/bl2/ext/mcuboot/bootutil/src/loader.c
+++ b/bl2/ext/mcuboot/bootutil/src/loader.c
@@ -168,29 +168,28 @@
boot_verify_image_header(struct image_header *hdr)
{
uint32_t image_end;
+ uint32_t x;
if (hdr->ih_magic != IMAGE_MAGIC) {
return BOOT_EBADIMAGE;
}
/* Check input parameters against integer overflow */
- if (boot_add_uint32_overflow_check(hdr->ih_hdr_size, hdr->ih_img_size)) {
+ if (!boot_u32_safe_add(&image_end, hdr->ih_hdr_size, hdr->ih_img_size)) {
return BOOT_EBADIMAGE;
}
- image_end = hdr->ih_hdr_size + hdr->ih_img_size;
- if (boot_add_uint32_overflow_check(image_end, hdr->ih_protect_tlv_size)) {
+ if (!boot_u32_safe_add(&x, image_end, hdr->ih_protect_tlv_size)) {
return BOOT_EBADIMAGE;
}
-
#if MCUBOOT_RAM_LOADING
if (!(hdr->ih_flags & IMAGE_F_RAM_LOAD)) {
return BOOT_EBADIMAGE;
}
/* Check input parameters against integer overflow */
- if (boot_add_uint32_overflow_check(image_end, hdr->ih_load_addr)) {
+ if (!boot_u32_safe_add(&x, image_end, hdr->ih_load_addr)) {
return BOOT_EBADIMAGE;
}
#endif
@@ -2538,16 +2537,18 @@
static int
boot_verify_ram_loading_address(uint32_t img_dst, uint32_t img_sz)
{
+ uint32_t img_end_addr;
+
if (img_dst < IMAGE_EXECUTABLE_RAM_START) {
return BOOT_EBADIMAGE;
}
- if (boot_add_uint32_overflow_check(img_dst, img_sz)) {
+ if (!boot_u32_safe_add(&img_end_addr, img_dst, img_sz)) {
return BOOT_EBADIMAGE;
}
- if (img_dst + img_sz > IMAGE_EXECUTABLE_RAM_START +
- IMAGE_EXECUTABLE_RAM_SIZE) {
+ if (img_end_addr > (IMAGE_EXECUTABLE_RAM_START +
+ IMAGE_EXECUTABLE_RAM_SIZE)) {
return BOOT_EBADIMAGE;
}
diff --git a/bl2/ext/mcuboot/bootutil/src/tlv.c b/bl2/ext/mcuboot/bootutil/src/tlv.c
index 3665e82..742c930 100644
--- a/bl2/ext/mcuboot/bootutil/src/tlv.c
+++ b/bl2/ext/mcuboot/bootutil/src/tlv.c
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2019 JUUL Labs
+ * Copyright (c) 2019 Arm Limited.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -14,6 +15,12 @@
* limitations under the License.
*/
+/*
+ * Original code taken from mcuboot project at:
+ * https://github.com/JuulLabs-OSS/mcuboot
+ * Git SHA of the original version: ac55554059147fff718015be9f4bd3108123f50a
+ */
+
#include <stddef.h>
#include <string.h>
@@ -71,7 +78,9 @@
it->type = type;
it->prot = prot;
it->prot_end = off_ + it->hdr->ih_protect_tlv_size;
- it->tlv_end = off_ + it->hdr->ih_protect_tlv_size + info.it_tlv_tot;
+ if (!boot_u32_safe_add(&(it->tlv_end), it->prot_end, info.it_tlv_tot)) {
+ return -1;
+ }
// position on first TLV
it->tlv_off = off_ + sizeof(info);
return 0;
@@ -121,11 +130,18 @@
}
*off = it->tlv_off + sizeof(tlv);
*len = tlv.it_len;
- it->tlv_off += sizeof(tlv) + tlv.it_len;
+
+ if (!boot_u32_safe_add(&(it->tlv_off), *off, *len)) {
+ return -1;
+ }
+
return 0;
}
- it->tlv_off += sizeof(tlv) + tlv.it_len;
+ if (!boot_u32_safe_add(&(it->tlv_off), it->tlv_off,
+ sizeof(tlv) + tlv.it_len)) {
+ return -1;
+ }
}
return 1;
diff --git a/bl2/src/boot_record.c b/bl2/src/boot_record.c
index 63c6181..7585251 100644
--- a/bl2/src/boot_record.c
+++ b/bl2/src/boot_record.c
@@ -298,6 +298,7 @@
struct shared_data_tlv_entry tlv_entry = {0};
struct tfm_boot_data *boot_data;
uint8_t *next_tlv;
+ uint16_t boot_data_size;
uintptr_t tlv_end, offset;
boot_data = (struct tfm_boot_data *)BOOT_TFM_SHARED_DATA_BASE;
@@ -334,15 +335,13 @@
tlv_entry.tlv_type = SET_TLV_TYPE(major_type, minor_type);
tlv_entry.tlv_len = SHARED_DATA_ENTRY_SIZE(size);
- /* Verify integer overflow */
- if (boot_add_uint16_overflow_check(boot_data->header.tlv_tot_len,
- tlv_entry.tlv_len)) {
+ if (!boot_u16_safe_add(&boot_data_size, boot_data->header.tlv_tot_len,
+ tlv_entry.tlv_len)) {
return SHARED_MEMORY_GEN_ERROR;
}
/* Verify overflow of shared area */
- if ((boot_data->header.tlv_tot_len + tlv_entry.tlv_len) >
- BOOT_TFM_SHARED_DATA_SIZE){
+ if (boot_data_size > BOOT_TFM_SHARED_DATA_SIZE) {
return SHARED_MEMORY_OVERFLOW;
}