Boot: Check integer overflow
Validate the input parameters from users, which comes
in the image header and image metadata (TLV) section,
to avoid integer overflow.
Change-Id: I1d1a48e8dbda2ced2620aa9fb19fda3bfbd801ab
Signed-off-by: Tamas Ban <tamas.ban@arm.com>
diff --git a/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c b/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
index 08f2a3c..f83db99 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_misc.c
@@ -28,6 +28,7 @@
#include <string.h>
#include <inttypes.h>
#include <stddef.h>
+#include <stdbool.h>
#include "flash_map/flash_map.h"
#include "bootutil/image.h"
@@ -721,3 +722,33 @@
return 0;
}
#endif /* BOOT_IMAGE_NUMBER > 1 */
+
+/**
+ * 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 1235fc9..c861e6f 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
@@ -27,6 +27,7 @@
#ifndef H_BOOTUTIL_PRIV_
#define H_BOOTUTIL_PRIV_
+#include <stdbool.h>
#include "flash_map/flash_map.h"
#include "bootutil/bootutil.h"
#include "bootutil/image.h"
@@ -192,6 +193,7 @@
const struct flash_area *area;
boot_sector_t *sectors;
size_t num_sectors;
+ bool is_hdr_valid;
} imgs[BOOT_IMAGE_NUMBER][BOOT_NUM_SLOTS];
struct {
@@ -230,6 +232,8 @@
int boot_is_version_sufficient(struct image_version *req,
struct image_version *ver);
#endif
+bool boot_add_uint32_overflow_check(uint32_t a, uint32_t b);
+bool boot_add_uint16_overflow_check(uint16_t a, uint16_t b);
/*
* Accessors for the contents of struct boot_loader_state.
@@ -238,6 +242,7 @@
/* These are macros so they can be used as lvalues. */
#define BOOT_IMG(state, slot) ((state)->imgs[current_image][(slot)])
#define BOOT_IMG_AREA(state, slot) (BOOT_IMG(state, slot).area)
+#define BOOT_IMG_HDR_IS_VALID(state, slot) (BOOT_IMG(state, slot).is_hdr_valid)
#define BOOT_SCRATCH_AREA(state) ((state)->scratch.area)
#define BOOT_WRITE_SZ(state) ((state)->write_sz)
#define BOOT_SWAP_TYPE(state) ((state)->swap_type[current_image])
diff --git a/bl2/ext/mcuboot/bootutil/src/image_validate.c b/bl2/ext/mcuboot/bootutil/src/image_validate.c
index e62455c..77b079c 100644
--- a/bl2/ext/mcuboot/bootutil/src/image_validate.c
+++ b/bl2/ext/mcuboot/bootutil/src/image_validate.c
@@ -210,6 +210,9 @@
if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
return BOOT_EBADMAGIC;
}
+ if (boot_add_uint32_overflow_check(off, (info.it_tlv_tot + sizeof(info)))) {
+ return -1;
+ }
end = off + info.it_tlv_tot;
off += sizeof(info);
@@ -217,6 +220,9 @@
* Traverse through all of the TLVs, performing any checks we know
* and are able to do.
*/
+ if (boot_add_uint32_overflow_check(load_address, end)) {
+ return -1;
+ }
while (off < end) {
tlv = *((struct image_tlv *)(load_address + off));
tlv_sz = sizeof(tlv);
@@ -239,7 +245,7 @@
}
/* Avoid integer overflow. */
- if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+ if (boot_add_uint32_overflow_check(off, (sizeof(tlv) + tlv.it_len))) {
/* Potential overflow. */
break;
} else {
@@ -330,7 +336,8 @@
}
/* Avoid integer overflow. */
- if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+ if (boot_add_uint32_overflow_check(off, (sizeof(tlv) + tlv.it_len)))
+ {
/* Potential overflow. */
break;
} else {
@@ -395,6 +402,9 @@
if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
return BOOT_EBADMAGIC;
}
+ if (boot_add_uint32_overflow_check(off, (info.it_tlv_tot + sizeof(info)))) {
+ return -1;
+ }
end = off + info.it_tlv_tot;
off += sizeof(info);
@@ -513,7 +523,7 @@
}
/* Avoid integer overflow. */
- if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+ if (boot_add_uint32_overflow_check(off, (sizeof(tlv) + tlv.it_len))) {
/* Potential overflow. */
break;
} else {
diff --git a/bl2/ext/mcuboot/bootutil/src/loader.c b/bl2/ext/mcuboot/bootutil/src/loader.c
index abf88df..0d741ff 100644
--- a/bl2/ext/mcuboot/bootutil/src/loader.c
+++ b/bl2/ext/mcuboot/bootutil/src/loader.c
@@ -155,6 +155,45 @@
(state)->image_ok)
#endif /* !MCUBOOT_NO_SWAP && !MCUBOOT_RAM_LOADING */
+/*
+ * \brief Verifies the image header: magic value, flags, integer overflow.
+ *
+ * \retval 0
+ * \retval BOOT_EBADIMAGE
+ */
+static int
+boot_verify_image_header(struct image_header *hdr)
+{
+ uint32_t image_end;
+
+ 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)) {
+ 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)) {
+ 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)) {
+ return BOOT_EBADIMAGE;
+ }
+#endif
+
+ return 0;
+}
static int
boot_read_image_header(int slot, struct image_header *out_hdr)
@@ -176,7 +215,8 @@
goto done;
}
- rc = 0;
+ rc = boot_verify_image_header(out_hdr);
+ BOOT_IMG_HDR_IS_VALID(&boot_data, slot) = (rc == 0);
done:
flash_area_close(fap);
@@ -338,8 +378,8 @@
goto out;
}
- if ((hdr->ih_magic != IMAGE_MAGIC ||
- boot_image_check(hdr, fap, bs) != 0)) {
+ if ((!BOOT_IMG_HDR_IS_VALID(&boot_data, slot)) ||
+ (boot_image_check(hdr, fap, bs) != 0)) {
if (slot != BOOT_PRIMARY_SLOT) {
rc = flash_area_erase(fap, 0, fap->fa_size);
if(rc != 0) {
@@ -1340,16 +1380,13 @@
* will be used to determine the amount of sectors to swap.
*/
hdr = boot_img_hdr(&boot_data, BOOT_PRIMARY_SLOT);
- if (hdr->ih_magic == IMAGE_MAGIC) {
- rc = boot_read_image_size(BOOT_PRIMARY_SLOT, hdr, ©_size);
- assert(rc == 0);
- }
+ rc = boot_read_image_size(BOOT_PRIMARY_SLOT, hdr, ©_size);
+ assert(rc == 0);
hdr = boot_img_hdr(&boot_data, BOOT_SECONDARY_SLOT);
- if (hdr->ih_magic == IMAGE_MAGIC) {
- rc = boot_read_image_size(BOOT_SECONDARY_SLOT, hdr, &size);
- assert(rc == 0);
- }
+ rc = boot_read_image_size(BOOT_SECONDARY_SLOT, hdr, &size);
+ assert(rc == 0);
+
if (size > copy_size) {
copy_size = size;
@@ -1569,11 +1606,14 @@
rc = BOOT_EBADIMAGE;
goto done;
}
+ if (boot_add_uint32_overflow_check(off, (info.it_tlv_tot + sizeof(info)))) {
+ return -1;
+ }
end = off + info.it_tlv_tot;
off += sizeof(info);
/* Traverse through all of the TLVs to find the dependency TLVs. */
- for (; off < end; off += sizeof(tlv) + tlv.it_len) {
+ while(off < end) {
rc = flash_area_read(fap, off, &tlv, sizeof(tlv));
if (rc != 0) {
rc = BOOT_EFLASH;
@@ -1614,6 +1654,13 @@
*/
break;
}
+ /* Avoid integer overflow. */
+ if (boot_add_uint32_overflow_check(off, (sizeof(tlv) + tlv.it_len))) {
+ /* Potential overflow. */
+ return BOOT_EBADIMAGE;
+ } else {
+ off += sizeof(tlv) + tlv.it_len;
+ }
}
done:
@@ -2095,11 +2142,8 @@
* onto an empty flash chip. At least do a basic sanity check that
* the magic number on the image is OK.
*/
- if (BOOT_IMG(&boot_data, BOOT_PRIMARY_SLOT).hdr.ih_magic !=
- IMAGE_MAGIC) {
- BOOT_LOG_ERR("bad image magic 0x%lx; Image=%u", (unsigned long)
- &boot_img_hdr(&boot_data,BOOT_PRIMARY_SLOT)->ih_magic,
- current_image);
+ if (!BOOT_IMG_HDR_IS_VALID(&boot_data, slot)) {
+ BOOT_LOG_ERR("Invalid image header Image=%u", current_image);
rc = BOOT_EBADIMAGE;
goto out;
}
@@ -2265,7 +2309,7 @@
continue;
}
- if (hdr->ih_magic == IMAGE_MAGIC) {
+ if (BOOT_IMG_HDR_IS_VALID(&boot_data, slot)) {
if (slot_state.magic == BOOT_MAGIC_GOOD ||
slot_state.image_ok == BOOT_FLAG_SET) {
/* Valid cases: