Peer review fixes

Author: dgarske

docs/Targets.md

@@ -1257,7 +1257,7 @@ MACHINE=mpfs-video-kit bitbake mchp-base-image-sdk
 
 Build images are output to: `./tmp-glibc/deploy/images/mpfs-video-kit/`
 
-#### Custom FIT image, signing and coping to SDCard
+#### Custom FIT image, signing and copying to SDCard
 
 wolfBoot can either decompress a gzipped kernel at boot time (`GZIP=1`,
 the default for `polarfire_mpfs250.config` and `polarfire_mpfs250_qspi.config`)
@@ -3455,6 +3455,13 @@ and the `bl31`/`fsbl` versus `bl31`/`plm` boot chain. Set `GZIP=0` in
 `.config` if you want to keep using an uncompressed `Image` plus
 `compression = "none"`.
 
+The decompressed-output bound for any single FIT subimage defaults to
+`WOLFBOOT_FIT_MAX_DECOMP = 256 MB`. Override per target via
+`CFLAGS+=-DWOLFBOOT_FIT_MAX_DECOMP=...` if a kernel/ramdisk legitimately
+expands beyond that. The outer wolfBoot signature still authenticates the
+entire FIT; this cap is defense-in-depth against a malformed-but-signed
+stream.
+
 **FIT ramdisk (initramfs)**
 
 When PetaLinux is built with `INITRAMFS_IMAGE_BUNDLE = "0"` the rootfs cpio
@@ -3497,6 +3504,7 @@ images {
         data = /incbin/("rootfs.cpio.gz");
         type = "ramdisk";
         compression = "gzip";  /* or "none" */
+        load = <0x40000000>;   /* required for decompression / relocation */
         hash-1 { algo = "sha256"; };
     };
 };

src/fdt.c

@@ -40,6 +40,16 @@
 #endif
 #endif /* WOLFBOOT_GZIP */
 
+/* Default upper bound on a single FIT subimage's decompressed size.
+ * The outer wolfBoot signature already authenticates the FIT, but a
+ * concrete cap defends against a malformed-but-signed FIT scribbling
+ * across unrelated memory. Override per target via:
+ *   CFLAGS+=-DWOLFBOOT_FIT_MAX_DECOMP=...
+ */
+#ifndef WOLFBOOT_FIT_MAX_DECOMP
+#define WOLFBOOT_FIT_MAX_DECOMP (256U * 1024U * 1024U)
+#endif
+
 uint32_t cpu_to_fdt32(uint32_t x)
 {
 #ifdef BIG_ENDIAN_ORDER
@@ -911,6 +921,9 @@ static int fit_verify_hash(const void *fdt, int img_off,
     int hash_off, len = 0;
     const char *algo = NULL;
     const uint8_t *value = NULL;
+#if defined(WOLFBOOT_HASH_SHA256) || defined(WOLFBOOT_HASH_SHA384)
+    int did_init = 0;
+#endif
 #ifdef WOLFBOOT_HASH_SHA256
     wc_Sha256 sha256_ctx;
     uint8_t sha256_digest[WC_SHA256_DIGEST_SIZE];
@@ -950,14 +963,20 @@ static int fit_verify_hash(const void *fdt, int img_off,
         }
         if (ret == 0) {
             ret = wc_InitSha256(&sha256_ctx);
+            if (ret == 0) {
+                did_init = 1;
+            }
         }
         if (ret == 0) {
             ret = wc_Sha256Update(&sha256_ctx, data, (word32)data_len);
         }
         if (ret == 0) {
             ret = wc_Sha256Final(&sha256_ctx, sha256_digest);
         }
-        wc_Sha256Free(&sha256_ctx);
+        if (did_init) {
+            wc_Sha256Free(&sha256_ctx);
+            did_init = 0;
+        }
         if (ret != 0) {
             wolfBoot_printf("FIT hash-1 (sha256): wc_Sha256 failed rc=%d\n",
                 ret);
@@ -982,14 +1001,20 @@ static int fit_verify_hash(const void *fdt, int img_off,
         }
         if (ret == 0) {
             ret = wc_InitSha384(&sha384_ctx);
+            if (ret == 0) {
+                did_init = 1;
+            }
         }
         if (ret == 0) {
             ret = wc_Sha384Update(&sha384_ctx, data, (word32)data_len);
         }
         if (ret == 0) {
             ret = wc_Sha384Final(&sha384_ctx, sha384_digest);
         }
-        wc_Sha384Free(&sha384_ctx);
+        if (did_init) {
+            wc_Sha384Free(&sha384_ctx);
+            did_init = 0;
+        }
         if (ret != 0) {
             wolfBoot_printf("FIT hash-1 (sha384): wc_Sha384 failed rc=%d\n",
                 ret);
@@ -1060,6 +1085,14 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
                 return NULL;
 #endif
             }
+            else if (comp != NULL && complen > 0
+                     && strcmp(comp, "none") != 0) {
+                /* Unknown compression scheme; fail closed rather than
+                 * silently memcpy compressed bytes as raw. */
+                wolfBoot_printf("FIT: subimage '%s' has unsupported "
+                    "compression=\"%s\"\n", image, comp);
+                return NULL;
+            }
             else {
                 wolfBoot_printf("Loading Image %s: %p -> %p (%d bytes)\n",
                     image, data, load, len);
@@ -1093,7 +1126,7 @@ void* fit_load_image_ex(void* fdt, const char* image, int* lenp,
 
 void* fit_load_image(void* fdt, const char* image, int* lenp)
 {
-    return fit_load_image_ex(fdt, image, lenp, 0xFFFFFFFFU);
+    return fit_load_image_ex(fdt, image, lenp, WOLFBOOT_FIT_MAX_DECOMP);
 }
 
 #endif /* (MMU || WOLFBOOT_FDT) && !BUILD_LOADER_STAGE1 */

tools/unit-tests/unit-gzip.c

@@ -35,7 +35,7 @@
 #include "gzip.h"
 
 /* Pull in the implementation under test directly */
-#include "gzip.c"
+#include "../../src/gzip.c"
 
 /* ------------------------------------------------------------------------- */
 /* Helpers: gzip(1) on the host produces our test fixtures                   */
@@ -45,41 +45,47 @@ static uint8_t *gz_compress_buf(const uint8_t *in, size_t in_len, size_t *out_le
 {
     char in_path[64], out_path[64];
     char cmd[256];
-    FILE *f;
+    FILE *f = NULL;
     long n;
-    uint8_t *buf;
+    uint8_t *buf = NULL;
 
     snprintf(in_path,  sizeof(in_path),  "/tmp/wb-gz-in-%d.bin",  getpid());
     snprintf(out_path, sizeof(out_path), "/tmp/wb-gz-out-%d.gz",  getpid());
 
     f = fopen(in_path, "wb");
-    if (f == NULL) return NULL;
+    if (f == NULL) goto cleanup;
     if (in_len > 0) {
         if (fwrite(in, 1, in_len, f) != in_len) {
             fclose(f);
-            return NULL;
+            f = NULL;
+            goto cleanup;
         }
     }
     fclose(f);
+    f = NULL;
 
     snprintf(cmd, sizeof(cmd), "gzip -nc %s > %s", in_path, out_path);
-    if (system(cmd) != 0) return NULL;
+    if (system(cmd) != 0) goto cleanup;
 
     f = fopen(out_path, "rb");
-    if (f == NULL) return NULL;
-    fseek(f, 0, SEEK_END);
+    if (f == NULL) goto cleanup;
+    if (fseek(f, 0, SEEK_END) != 0) goto cleanup;
     n = ftell(f);
-    fseek(f, 0, SEEK_SET);
+    if (n < 0) goto cleanup;
+    if (fseek(f, 0, SEEK_SET) != 0) goto cleanup;
     buf = (uint8_t*)malloc((size_t)n);
-    if (buf == NULL) { fclose(f); return NULL; }
+    if (buf == NULL) goto cleanup;
     if (fread(buf, 1, (size_t)n, f) != (size_t)n) {
-        free(buf); fclose(f);
-        return NULL;
+        free(buf);
+        buf = NULL;
+        goto cleanup;
     }
-    fclose(f);
+    *out_len = (size_t)n;
+
+cleanup:
+    if (f != NULL) fclose(f);
     unlink(in_path);
     unlink(out_path);
-    *out_len = (size_t)n;
     return buf;
 }