Fix integer overflows in BL1 FWU code

Before adding a base address and a size to compute the end
address of an image to copy or authenticate, check this
won't result in an integer overflow. If it does then consider
the input arguments are invalid.

As a result, bl1_plat_mem_check() can now safely assume the
end address (computed as the sum of the base address and size
of the memory region) doesn't overflow, as the validation is
done upfront in bl1_fwu_image_copy/auth(). A debug assertion
has been added nonetheless in the ARM implementation in order
to help catching such problems, should bl1_plat_mem_check()
be called in a different context in the future.

Fixes TFV-1: Malformed Firmware Update SMC can result in copy
of unexpectedly large data into secure memory

Change-Id: I8b8f8dd4c8777705722c7bd0e8b57addcba07e25
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Signed-off-by: Dan Handley <dan.handley@arm.com>

diff --git a/bl1/bl1_fwu.c b/bl1/bl1_fwu.c
index 7ef184c11deb52c9358c223fe164eb31b2a57b67..1cc7daf62912bef286b5417a7e1dce89d4f69a0c 100644 (file)
--- a/bl1/bl1_fwu.c
+++ b/bl1/bl1_fwu.c
@@ -41,6 +41,7 @@
 #include <platform_def.h>
 #include <smcc_helpers.h>
 #include <string.h>
+#include <utils.h>
 #include "bl1_private.h"
 
 /*
@@ -151,7 +152,8 @@ static int bl1_fwu_image_copy(unsigned int image_id,
                return -EPERM;
        }
 
-       if ((!image_src) || (!block_size)) {
+       if ((!image_src) || (!block_size) ||
+           check_uptr_overflow(image_src, block_size - 1)) {
                WARN("BL1-FWU: Copy not allowed due to invalid image source"
                        " or block size\n");
                return -ENOMEM;
@@ -192,11 +194,14 @@ static int bl1_fwu_image_copy(unsigned int image_id,
                        return -ENOMEM;
                }
 #else
-               /* Find out how much free trusted ram remains after BL1 load */
+               /*
+                * Check the image will fit into the free trusted RAM after BL1
+                * load.
+                */
                const meminfo_t *mem_layout = bl1_plat_sec_mem_layout();
-               if ((image_desc->image_info.image_base < mem_layout->free_base) ||
-                        (image_desc->image_info.image_base + image_size >
-                         mem_layout->free_base + mem_layout->free_size)) {
+               if (!is_mem_free(mem_layout->free_base, mem_layout->free_size,
+                                       image_desc->image_info.image_base,
+                                       image_size)) {
                        WARN("BL1-FWU: Copy not allowed due to insufficient"
                             " resources.\n");
                        return -ENOMEM;
@@ -290,7 +295,8 @@ static int bl1_fwu_image_auth(unsigned int image_id,
                base_addr = image_desc->image_info.image_base;
                total_size = image_desc->image_info.image_size;
        } else {
-               if ((!image_src) || (!image_size)) {
+               if ((!image_src) || (!image_size) ||
+                   check_uptr_overflow(image_src, image_size - 1)) {
                        WARN("BL1-FWU: Auth not allowed due to invalid"
                                " image source/size\n");
                        return -ENOMEM;

diff --git a/plat/arm/common/arm_bl1_fwu.c b/plat/arm/common/arm_bl1_fwu.c
index 2a18d34130aabb60e90d1b7e905a39918b8b2c42..da4107b6a6ce6064ba07e806feadca5541f0fc3c 100644 (file)
--- a/plat/arm/common/arm_bl1_fwu.c
+++ b/plat/arm/common/arm_bl1_fwu.c
@@ -35,7 +35,7 @@
 #include <plat_arm.h>
 #include <platform_def.h>
 #include <tbbr_img_desc.h>
-
+#include <utils.h>
 
 /* Struct to keep track of usable memory */
 typedef struct bl1_mem_info {
@@ -76,6 +76,12 @@ int bl1_plat_mem_check(uintptr_t mem_base,
 
        assert(mem_base);
        assert(mem_size);
+       /*
+        * The caller of this function is responsible for checking upfront that
+        * the end address doesn't overflow. We double-check this in debug
+        * builds.
+        */
+       assert(!check_uptr_overflow(mem_base, mem_size - 1));
 
        /*
         * Check the given image source and size.