Merge pull request #693 from danielinux/fix-delta-boundary-checks

[delta] Add bounds checks for delta image

Author: dgarske

src/delta.c

@@ -106,6 +106,7 @@ int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len)
     uint32_t src_off;
     uint16_t sz;
     uint32_t copy_sz;
+    uint32_t resume_sz;
     if (!ctx)
         return -1;
     if (len < BLOCK_HDR_SIZE)
@@ -115,10 +116,13 @@ int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len)
         uint8_t *pp = patch_read_cache(ctx);
         if (ctx->matching) {
             /* Resume matching block from previous sector */
-            sz = ctx->blk_sz;
-            if (sz > len)
-                sz = len;
-            memcpy(dst + dst_off, ctx->src_base + ctx->blk_off, sz);
+            resume_sz = ctx->blk_sz;
+            if (resume_sz > len)
+                resume_sz = len;
+            if (ctx->blk_off > ctx->src_size ||
+                    resume_sz > ctx->src_size - ctx->blk_off)
+                return -1;
+            memcpy(dst + dst_off, ctx->src_base + ctx->blk_off, resume_sz);
             if (ctx->blk_sz > len) {
                 ctx->blk_sz -= len;
                 ctx->blk_off += len;
@@ -127,7 +131,7 @@ int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len)
                 ctx->blk_sz = 0;
                 ctx->matching = 0;
             }
-            dst_off += sz;
+            dst_off += resume_sz;
             continue;
         }
         if (*pp == ESC) {
@@ -142,6 +146,9 @@ int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len)
                 src_off = (hdr->off[0] << 16) + (hdr->off[1] << 8) +
                     hdr->off[2];
                 sz = (hdr->sz[0] << 8) + hdr->sz[1];
+                if (src_off > ctx->src_size ||
+                        sz > ctx->src_size - src_off)
+                    return -1;
                 ctx->matching = 1;
                 if (sz > (len - dst_off)) {
                     copy_sz = len - dst_off;

src/update_flash.c

@@ -441,6 +441,11 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot,
     uint16_t base_hash_sz;
     uint8_t *base_hash;
 
+    if (boot->fw_size == 0) {
+        /* Resume after powerfail can leave boot header erased; bound by partition size. */
+        boot->fw_size = WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE;
+    }
+
     /* Use biggest size for the swap */
     total_size = boot->fw_size + IMAGE_HEADER_SIZE;
     if ((update->fw_size + IMAGE_HEADER_SIZE) > total_size)

tools/unit-tests/unit-delta.c

@@ -48,6 +48,71 @@ START_TEST(test_wb_patch_init_invalid)
 }
 END_TEST
 
+START_TEST(test_wb_patch_src_bounds_invalid)
+{
+    WB_PATCH_CTX patch_ctx;
+    uint8_t src[SRC_SIZE] = {0};
+    uint8_t patch[PATCH_SIZE] = {0};
+    uint8_t dst[DELTA_BLOCK_SIZE] = {0};
+    int ret;
+
+    /* ESC + header with src_off beyond src_size */
+    patch[0] = ESC;
+    patch[1] = 0x00; /* off[0] */
+    patch[2] = 0x10; /* off[1] -> 0x0010FF */
+    patch[3] = 0xFF; /* off[2] */
+    patch[4] = 0x00; /* sz[0] */
+    patch[5] = 0x10; /* sz[1] -> 16 */
+
+    ret = wb_patch_init(&patch_ctx, src, SRC_SIZE, patch, BLOCK_HDR_SIZE);
+    ck_assert_int_eq(ret, 0);
+
+    ret = wb_patch(&patch_ctx, dst, sizeof(dst));
+    ck_assert_int_eq(ret, -1);
+}
+END_TEST
+
+START_TEST(test_wb_patch_resume_bounds_invalid)
+{
+    WB_PATCH_CTX patch_ctx;
+    uint8_t src[SRC_SIZE] = {0};
+    uint8_t patch[PATCH_SIZE] = {0};
+    uint8_t dst[DELTA_BLOCK_SIZE] = {0};
+    int ret;
+
+    ret = wb_patch_init(&patch_ctx, src, SRC_SIZE, patch, BLOCK_HDR_SIZE);
+    ck_assert_int_eq(ret, 0);
+
+    patch_ctx.matching = 1;
+    patch_ctx.blk_off = SRC_SIZE + 1;
+    patch_ctx.blk_sz = 4;
+
+    ret = wb_patch(&patch_ctx, dst, sizeof(dst));
+    ck_assert_int_eq(ret, -1);
+}
+END_TEST
+
+START_TEST(test_wb_patch_resume_large_len)
+{
+    WB_PATCH_CTX patch_ctx;
+    uint8_t src[SRC_SIZE] = {0};
+    uint8_t patch[PATCH_SIZE] = {0};
+    uint8_t dst[DST_SIZE] = {0};
+    uint32_t len = 70000;
+    int ret;
+
+    src[0] = 0xA5;
+    ret = wb_patch_init(&patch_ctx, src, SRC_SIZE, patch, BLOCK_HDR_SIZE);
+    ck_assert_int_eq(ret, 0);
+
+    patch_ctx.matching = 1;
+    patch_ctx.blk_off = 0;
+    patch_ctx.blk_sz = len;
+
+    ret = wb_patch(&patch_ctx, dst, len);
+    ck_assert_int_eq(ret, -1);
+}
+END_TEST
 
 START_TEST(test_wb_diff_init_invalid)
 {
@@ -162,6 +227,9 @@ Suite *patch_diff_suite(void)
 
     tcase_add_test(tc_wolfboot_delta, test_wb_patch_init_invalid);
     tcase_add_test(tc_wolfboot_delta, test_wb_diff_init_invalid);
+    tcase_add_test(tc_wolfboot_delta, test_wb_patch_src_bounds_invalid);
+    tcase_add_test(tc_wolfboot_delta, test_wb_patch_resume_bounds_invalid);
+    tcase_add_test(tc_wolfboot_delta, test_wb_patch_resume_large_len);
     tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff);
     suite_add_tcase(s, tc_wolfboot_delta);