cros_read_bootflow() locates the kernel preamble with

        preamble = (void *)hdr + hdr->keyblock_size;

where hdr is a PROBE_SIZE (4KB) buffer holding the start of the kernel
partition and keyblock_size is a 32-bit value read from the disk. The
only field checked before this is the keyblock magic, so a corrupt or
malicious partition can carry an arbitrary keyblock_size and make every
subsequent preamble access read far outside the buffer, crashing U-Boot
or fetching garbage that then drives allocation sizes and disk offsets.

Reject the partition if the keyblock claims to be smaller than its own
header, or if the preamble does not lie entirely within the probed
area.

Also require the computed kernel body offset (keyblock_size plus
preamble_size) to lie beyond the probed area, as cros_read_kernel()
already demands just before reading the body. Rejecting such invalid
metadata during scanning means no bootflow is created for it, and the
single comparison also catches a preamble_size that wraps the 32-bit
sum, since a wrapped value is necessarily smaller than keyblock_size
and therefore below PROBE_SIZE.

Fixes: 3257835e5640 ("bootstd: cros: Decode some kernel preamble fields")
Signed-off-by: Aristo Chen <[email protected]>
---
 boot/bootmeth_cros.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
index f894ed78e3a..685d5335e86 100644
--- a/boot/bootmeth_cros.c
+++ b/boot/bootmeth_cros.c
@@ -369,6 +369,7 @@ static int cros_read_bootflow(struct udevice *dev, struct 
bootflow *bflow)
        struct vb2_keyblock *hdr;
        const char *uuid = NULL;
        struct cros_priv *priv;
+       ulong body_offset;
        int ret;
 
        log_debug("starting, part=%x\n", bflow->part);
@@ -380,6 +381,31 @@ static int cros_read_bootflow(struct udevice *dev, struct 
bootflow *bflow)
                return log_msg_ret("scan", ret);
        }
 
+       /*
+        * Make sure that the preamble, which follows the keyblock, lies within
+        * the data read by scan_part(), since keyblock_size comes from the
+        * disk and cannot be trusted
+        */
+       if (hdr->keyblock_size < sizeof(*hdr) ||
+           hdr->keyblock_size > PROBE_SIZE - sizeof(*preamble)) {
+               log_debug("- invalid keyblock size %x\n", hdr->keyblock_size);
+               free(hdr);
+               return log_msg_ret("kblk", -ERANGE);
+       }
+       preamble = (void *)hdr + hdr->keyblock_size;
+
+       /*
+        * The kernel body must start beyond the probed area, as
+        * cros_read_kernel() requires; this also rejects a preamble_size
+        * that wraps the 32-bit sum
+        */
+       body_offset = hdr->keyblock_size + preamble->preamble_size;
+       if (body_offset < PROBE_SIZE) {
+               log_debug("- invalid body offset %lx\n", body_offset);
+               free(hdr);
+               return log_msg_ret("bod", -ERANGE);
+       }
+
        priv = malloc(sizeof(struct cros_priv));
        if (!priv) {
                free(hdr);
@@ -391,8 +417,7 @@ static int cros_read_bootflow(struct udevice *dev, struct 
bootflow *bflow)
                  (ulong)map_to_sysmem(hdr));
 
        /* Grab a few things from the preamble */
-       preamble = (void *)hdr + hdr->keyblock_size;
-       priv->body_offset = hdr->keyblock_size + preamble->preamble_size;
+       priv->body_offset = body_offset;
        priv->part_start = info.start;
 
        /* Now read everything we can learn about kernel */
-- 
2.43.0

Reply via email to