Fixes to delta updates, reinforced size checks, improved hardening (wolfSSL#716)

Fixes to delta updates, reinforced size checks, improved hardening

Author: dgarske

src/boot_x86_fsp.c

@@ -82,6 +82,9 @@ const uint8_t __attribute__((section(".sig_wolfboot_raw")))
 #define FSP_STATUS_RESET_REQUIRED_WARM  0x40000002
 #define MEMORY_4GB (4ULL * 1024 * 1024 * 1024)
 #define ENDLINE "\r\n"
+/* Standard PCI capabilities live in conventional config space at 0x40-0xFC,
+ * on 4-byte alignment, so there are at most 48 distinct capability headers. */
+#define PCI_MAX_STANDARD_CAPABILITIES 48
 
 
 /* compile time alignment checks */
@@ -257,13 +260,23 @@ static void jump_into_wolfboot(void)
 /* The image needs to be already verified */
 int wolfBoot_image_measure(uint8_t *image)
 {
-    uint16_t hash_len;
-    uint8_t *hash;
+    struct wolfBoot_image img;
+    int ret;
 
-    hash_len = wolfBoot_find_header(image + IMAGE_HEADER_OFFSET,
-                                    WOLFBOOT_SHA_HDR, &hash);
-    wolfBoot_print_hexstr(hash, hash_len, 0);
-    return wolfBoot_tpm2_extend(WOLFBOOT_MEASURED_PCR_A, hash, __LINE__);
+    memset(&img, 0, sizeof(img));
+    ret = wolfBoot_open_image_address(&img, image);
+    if (ret != 0) {
+        return ret;
+    }
+
+    ret = wolfBoot_verify_integrity(&img);
+    if (ret != 0 || img.sha_hash == NULL) {
+        return -1;
+    }
+
+    wolfBoot_print_hexstr(img.sha_hash, WOLFBOOT_SHA_DIGEST_SIZE, 0);
+    return wolfBoot_tpm2_extend(WOLFBOOT_MEASURED_PCR_A, img.sha_hash,
+        __LINE__);
 }
 #endif /* WOLFBOOT_MEASURED_BOOT */
 
@@ -332,19 +345,21 @@ static int pci_get_capability(uint8_t bus, uint8_t dev, uint8_t fun,
                               uint8_t cap_id, uint8_t *cap_off)
 {
     uint8_t r8, id;
+    uint8_t cap_count = 0;
     uint32_t r32;
 
     r32 = pci_config_read16(bus, dev, fun, PCI_STATUS_OFFSET);
     if (!(r32 & PCI_STATUS_CAP_LIST))
         return -1;
     r8 = pci_config_read8(bus, dev, fun, PCI_CAP_OFFSET);
-    while (r8 != 0) {
+    while ((r8 != 0) && (cap_count < PCI_MAX_STANDARD_CAPABILITIES)) {
         id = pci_config_read8(bus, dev, fun, r8);
         if (id == cap_id) {
             *cap_off = r8;
             return 0;
         }
         r8 = pci_config_read8(bus, dev, fun, r8 + 1);
+        cap_count++;
     }
     return -1;
 }

src/delta.c

@@ -135,13 +135,17 @@ int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len)
             continue;
         }
         if (*pp == ESC) {
+            if ((ctx->patch_size - ctx->p_off) < 2)
+                return -1;
             if (*(pp + 1) == ESC) {
                 *(dst + dst_off) = ESC;
                 /* Two bytes of the patch have been consumed to produce ESC */
                 ctx->p_off += 2;
                 dst_off++;
                 continue;
             } else {
+                if ((ctx->patch_size - ctx->p_off) < BLOCK_HDR_SIZE)
+                    return -1;
                 hdr = (struct block_hdr *)pp;
                 src_off = (hdr->off[0] << 16) + (hdr->off[1] << 8) +
                     hdr->off[2];

src/gpt.c

@@ -34,6 +34,23 @@
 
 #include "gpt.h"
 
+static uint32_t gpt_crc32(const uint8_t *data, uint32_t len)
+{
+    uint32_t crc = 0xFFFFFFFFU;
+    uint32_t i;
+    uint32_t j;
+
+    for (i = 0; i < len; i++) {
+        crc ^= data[i];
+        for (j = 0; j < 8; j++) {
+            uint32_t mask = -(crc & 1U);
+            crc = (crc >> 1) ^ (0xEDB88320U & mask);
+        }
+    }
+
+    return ~crc;
+}
+
 /**
  * @brief Check MBR for protective GPT partition entry.
  *
@@ -98,6 +115,7 @@ int gpt_check_mbr_protective(const uint8_t *mbr_sector, uint32_t *gpt_lba)
 int gpt_parse_header(const uint8_t *sector, struct guid_ptable *hdr)
 {
     const struct guid_ptable *src;
+    struct guid_ptable tmp;
 
     if (sector == NULL || hdr == NULL) {
         return -1;
@@ -110,6 +128,16 @@ int gpt_parse_header(const uint8_t *sector, struct guid_ptable *hdr)
         return -1;
     }
 
+    if (src->hdr_size < 0x5C || src->hdr_size > GPT_SECTOR_SIZE) {
+        return -1;
+    }
+
+    memcpy(&tmp, src, sizeof(tmp));
+    tmp.hdr_crc32 = 0;
+    if (gpt_crc32((const uint8_t *)&tmp, src->hdr_size) != src->hdr_crc32) {
+        return -1;
+    }
+
     /* Copy header to output */
     memcpy(hdr, src, sizeof(struct guid_ptable));
 

src/image.c

@@ -58,6 +58,19 @@
 /* Globals */
 static uint8_t digest[WOLFBOOT_SHA_DIGEST_SIZE] XALIGNED(4);
 
+static int image_CT_compare(const uint8_t *expected, const uint8_t *actual,
+    uint32_t len)
+{
+    uint8_t diff = 0;
+    uint32_t i;
+
+    for (i = 0; i < len; i++) {
+        diff |= expected[i] ^ actual[i];
+    }
+
+    return diff == 0;
+}
+
 #if defined(WOLFBOOT_CERT_CHAIN_VERIFY) && \
     (defined(WOLFBOOT_ENABLE_WOLFHSM_CLIENT) || \
      defined(WOLFBOOT_ENABLE_WOLFHSM_SERVER))
@@ -367,6 +380,9 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot,
 static inline int DecodeAsn1Tag(const uint8_t* input, int inputSz, int* inOutIdx,
     int* tag_len, uint8_t tag)
 {
+    if (*inOutIdx < 0 || *inOutIdx >= inputSz || (*inOutIdx + 1) >= inputSz) {
+        return -1;
+    }
     if (input[*inOutIdx] != tag) {
         return -1;
     }
@@ -1492,6 +1508,12 @@ int wolfBoot_open_self_address(struct wolfBoot_image* img, uint8_t* hdr,
 
     img->hdr     = hdr;
     img->fw_size = wolfBoot_image_size(hdr);
+#ifdef WOLFBOOT_FIXED_PARTITIONS
+    if (img->fw_size > (WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE)) {
+        img->fw_size = WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE;
+        return -1;
+    }
+#endif
     img->fw_base = image;
     img->part    = PART_SELF;
     img->hdr_ok  = 1;
@@ -1518,7 +1540,7 @@ int wolfBoot_verify_integrity(struct wolfBoot_image *img)
         return -1;
     if (image_hash(img, digest) != 0)
         return -1;
-    if (memcmp(digest, stored_sha, stored_sha_len) != 0)
+    if (!image_CT_compare(digest, stored_sha, stored_sha_len))
         return -1;
     img->sha_ok = 1;
     img->sha_hash = stored_sha;
@@ -2366,6 +2388,21 @@ uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset,
 
 #if !defined(WOLFBOOT_NO_SIGN) && !defined(WOLFBOOT_RENESAS_SCEPROTECT)
 
+/* Compare fixed-size key hints without early exit to avoid leaking hash prefix
+ * matches through lookup timing. */
+static int keyslot_CT_hint_matches(const uint8_t *expected,
+    const uint8_t *actual)
+{
+    uint8_t diff = 0;
+    uint32_t i;
+
+    for (i = 0; i < WOLFBOOT_SHA_DIGEST_SIZE; i++) {
+        diff |= expected[i] ^ actual[i];
+    }
+
+    return diff == 0;
+}
+
 /**
  * @brief Get the key slot ID by SHA hash.
  *
@@ -2378,13 +2415,14 @@ uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset,
 int keyslot_id_by_sha(const uint8_t *hint)
 {
     int id;
+    int match_id = -1;
 
     for (id = 0; id < keystore_num_pubkeys(); id++) {
         key_hash(id, digest);
-        if (memcmp(digest, hint, WOLFBOOT_SHA_DIGEST_SIZE) == 0) {
-            return id;
+        if ((match_id < 0) && keyslot_CT_hint_matches(digest, hint)) {
+            match_id = id;
         }
     }
-    return -1;
+    return match_id;
 }
 #endif /* !WOLFBOOT_NO_SIGN && !WOLFBOOT_RENESAS_SCEPROTECT */

src/libwolfboot.c

@@ -107,8 +107,8 @@ int wolfBoot_initialize_encryption(void)
                              (2 * WOLFBOOT_SECTOR_SIZE)))
                              /* MAGIC (4B) + PART_FLAG (1B) + (N_SECTORS / 2) */
     #define START_FLAGS_OFFSET (ENCRYPT_TMP_SECRET_OFFSET - TRAILER_OVERHEAD)
-    #define SECTOR_FLAGS_SIZE WOLFBOOT_SECTOR_SIZE - (4 + 1 + \
-        ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE)
+    #define SECTOR_FLAGS_SIZE (WOLFBOOT_SECTOR_SIZE - (4 + 1 + \
+        ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE))
     /* MAGIC (4B) + PART_FLAG (1B) + ENCRYPT_KEY_SIZE + ENCRYPT_NONCE_SIZE */
 #else
     #define ENCRYPT_TMP_SECRET_OFFSET (WOLFBOOT_PARTITION_SIZE - (TRAILER_SKIP))

src/pkcs11_store.c

@@ -506,8 +506,8 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len)
            % WOLFBOOT_SECTOR_SIZE;
         sector_base = (uintptr_t)handle->buffer + handle->in_buffer_offset - in_sector_offset;
         in_sector_len = WOLFBOOT_SECTOR_SIZE - in_sector_offset;
-        if (in_sector_len > (uint32_t)len)
-            in_sector_len = len;
+        if (in_sector_len > (uint32_t)(len - written))
+            in_sector_len = len - written;
 
         /* Cache the corresponding sector */
         memcpy(cached_sector, (void *)(uintptr_t)sector_base, WOLFBOOT_SECTOR_SIZE);

src/psa_store.c

@@ -512,8 +512,8 @@ int wolfPSA_Store_Write(void* store, unsigned char* buffer, int len)
            % WOLFBOOT_SECTOR_SIZE;
         sector_base = (uintptr_t)handle->buffer + handle->in_buffer_offset - in_sector_offset;
         in_sector_len = WOLFBOOT_SECTOR_SIZE - in_sector_offset;
-        if (in_sector_len > (uint32_t)len)
-            in_sector_len = len;
+        if (in_sector_len > (uint32_t)(len - written))
+            in_sector_len = len - written;
 
         /* Cache the corresponding sector */
         memcpy(cached_sector, (void *)(uintptr_t)sector_base, WOLFBOOT_SECTOR_SIZE);

src/qspi_flash.c

@@ -417,8 +417,10 @@ int spi_flash_read(uint32_t address, void *data, int len)
 int spi_flash_write(uint32_t address, const void *data, int len)
 {
     int ret = 0;
+    int remaining = len;
     uint32_t xferSz, page, pages;
     uintptr_t addr;
+    uint8_t* ptr = (uint8_t*)data;
 
 #ifdef DEBUG_QSPI
     wolfBoot_printf("QSPI Flash Write: Len %d, %p -> 0x%x\n",
@@ -430,13 +432,12 @@ int spi_flash_write(uint32_t address, const void *data, int len)
     for (page = 0; page < pages; page++) {
         ret = qspi_write_enable();
         if (ret == 0) {
-            uint8_t* ptr;
-            xferSz = len;
-            if (xferSz > FLASH_PAGE_SIZE)
+            xferSz = (uint32_t)remaining;
+            if (xferSz > FLASH_PAGE_SIZE) {
                 xferSz = FLASH_PAGE_SIZE;
+            }
 
             addr = address + (page * FLASH_PAGE_SIZE);
-            ptr = ((uint8_t*)data + (page * FLASH_PAGE_SIZE));
 
             /* ------ Write Flash (page at a time) ------ */
             ret = qspi_transfer(QSPI_MODE_WRITE, FLASH_WRITE_CMD,
@@ -459,6 +460,8 @@ int spi_flash_write(uint32_t address, const void *data, int len)
                 break;
             }
             /* write disable is automatic */
+            remaining -= (int)xferSz;
+            ptr += xferSz;
         }
     }
 

src/string.c

@@ -361,7 +361,7 @@ void uart_vprintf(const char* fmt, va_list argp)
                 if (*fmtp == '0' && maxdigits == 0) {
                     zeropad = 1;
                 }
-                maxdigits <<= 8;
+                maxdigits *= 10;
                 maxdigits += (*fmtp - '0');
                 fmtp++;
             }

src/tpm.c

@@ -382,6 +382,8 @@ int wolfBoot_load_pubkey(const uint8_t* pubkey_hint, WOLFTPM2_KEY* pubKey,
           defined(WOLFBOOT_SIGN_RSA3072) || \
           defined(WOLFBOOT_SIGN_RSA4096)
         uint32_t inOutIdx = 0;
+        uint32_t exponent = 0;
+        uint32_t j;
         const uint8_t*n = NULL, *e = NULL;
         uint32_t nSz = 0, eSz = 0;
         if (key_type != AUTH_KEY_RSA2048 && key_type != AUTH_KEY_RSA3072 &&
@@ -395,10 +397,19 @@ int wolfBoot_load_pubkey(const uint8_t* pubkey_hint, WOLFTPM2_KEY* pubKey,
                 &e, &eSz  /* exponent */
             );
         }
+        if (rc == 0) {
+            if (eSz == 0 || eSz > sizeof(exponent))
+                rc = -1;
+        }
+        if (rc == 0) {
+            for (j = 0; j < eSz; j++) {
+                exponent = (exponent << 8) | e[j];
+            }
+        }
         if (rc == 0) {
             /* Load public key into TPM */
             rc = wolfTPM2_LoadRsaPublicKey_ex(&wolftpm_dev, pubKey,
-                n, nSz, *((uint32_t*)e),
+                n, nSz, exponent,
                 TPM_ALG_NULL, WOLFBOOT_TPM_HASH_ALG);
         }
     #else