[PATCH v1 0/3] fit: cipher: bounds checks on the ciphered image path

[PATCH v1 0/3] fit: cipher: bounds checks on the ciphered image path

[Pranav Rajendran]

Three fixes in the FIT cipher path, found while auditing for image-
controlled values used as buffer lengths without validation. They were
reported to the list earlier; these are the fixes.

Patch 1 is an out-of-bounds write. image_aes_decrypt() sizes its output
buffer with cipher_len but decrypts DIV_ROUND_UP(cipher_len, block)
blocks into it, so a 'data' length that is not block aligned writes past
the end of the allocation.

Patches 2 and 3 are the two halves of the unciphered size never being
checked: the 'data-size-unciphered' property is read without asking for
its length, and the value it yields is never related to the amount of
data that was actually decrypted. The second matters because that
property sits outside the per-image hash and signature, so it can be
changed on a signed FIT without disturbing verification.

I have kept them as separate patches since they are independent defects
with different consequences, but I am happy to squash 1 and 3 if you
would rather have one change per function.

checkpatch-clean, builds for sandbox (CONFIG_FIT_CIPHER=y) with no new
warnings at W=1, and no change in test/py results.

Pranav Rajendran (3):
  lib: aes: reject a ciphertext length that is not a whole number of
    blocks
  image-fit: check the length of the data-size-unciphered property
  lib: aes: reject an unciphered size larger than the ciphertext

 boot/image-fit.c      |  6 +++++-
 lib/aes/aes-decrypt.c | 21 ++++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v1 1/3] lib: aes: reject a ciphertext length that is not a whole number of blocks

[Pranav Rajendran]

image_aes_decrypt() allocates cipher_len bytes for the plaintext but
then asks aes_cbc_decrypt_blocks() to write
DIV_ROUND_UP(cipher_len, AES_BLOCK_LENGTH) blocks into it. For a
cipher_len that is not a multiple of AES_BLOCK_LENGTH the rounding up
adds one block, so the last block is written up to AES_BLOCK_LENGTH - 1
bytes past the end of the allocation, and read the same distance past
the end of the ciphertext.

cipher_len is the size of the image data in the FIT, so an image with a
'data' property whose length is not block aligned is enough to reach
this. The overflowing bytes are decryption output, i.e. they depend on
the key, but the length itself is not covered by anything that would
stop the image from being parsed this far.

A CBC ciphertext is a whole number of blocks by construction, so treat
anything else as a malformed image and reject it before allocating.
With that established, compute the block count with a plain division so
the buffer size and the write length cannot drift apart again.

Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
 lib/aes/aes-decrypt.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/aes/aes-decrypt.c b/lib/aes/aes-decrypt.c
index 741102a4723..85773a9c4f6 100644
--- a/lib/aes/aes-decrypt.c
+++ b/lib/aes/aes-decrypt.c
@@ -17,6 +17,16 @@ int image_aes_decrypt(struct image_cipher_info *info,
 	unsigned char key_exp[AES256_EXPAND_KEY_LENGTH];
 	unsigned int aes_blocks, key_len = info->cipher->key_len;
 
+	/*
+	 * The ciphertext is a whole number of AES blocks by construction, and
+	 * the decryption below writes one full block at a time, so anything
+	 * else would overflow the output buffer.
+	 */
+	if (!cipher_len || cipher_len % AES_BLOCK_LENGTH) {
+		printf("Invalid ciphertext length\n");
+		return -EINVAL;
+	}
+
 	*data = malloc(cipher_len);
 	if (!*data) {
 		printf("Can't allocate memory to decrypt\n");
@@ -30,7 +40,7 @@ int image_aes_decrypt(struct image_cipher_info *info,
 	aes_expand_key((u8 *)info->key, key_len, key_exp);
 
 	/* Calculate the number of AES blocks to encrypt. */
-	aes_blocks = DIV_ROUND_UP(cipher_len, AES_BLOCK_LENGTH);
+	aes_blocks = cipher_len / AES_BLOCK_LENGTH;
 
 	aes_cbc_decrypt_blocks(key_len, key_exp, (u8 *)info->iv,
 			       (u8 *)cipher, *data, aes_blocks);
-- 
2.50.1 (Apple Git-155)

[PATCH v1 2/3] image-fit: check the length of the data-size-unciphered property

[Pranav Rajendran]

fit_image_get_data_size_unciphered() passes NULL as fdt_getprop()'s
length argument, so it accepts a 'data-size-unciphered' property of any
size and then dereferences the first four bytes of it. A property
shorter than that is read past its end, and the bytes that follow it in
the FIT are returned to the caller as the unciphered size.

Ask for the length and require it to be exactly one fdt32_t, as the
binding describes.

Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
 boot/image-fit.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index ef90c5abd18..0b98cc242e3 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1040,11 +1040,15 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 				       size_t *data_size)
 {
 	const fdt32_t *val;
+	int len;
 
-	val = fdt_getprop(fit, noffset, "data-size-unciphered", NULL);
+	val = fdt_getprop(fit, noffset, "data-size-unciphered", &len);
 	if (!val)
 		return -ENOENT;
 
+	if (len != sizeof(*val))
+		return -EINVAL;
+
 	*data_size = (size_t)fdt32_to_cpu(*val);
 
 	return 0;
-- 
2.50.1 (Apple Git-155)

[PATCH v1 3/3] lib: aes: reject an unciphered size larger than the ciphertext

[Pranav Rajendran]

image_aes_decrypt() allocates cipher_len bytes, decrypts into them, and
then reports the plaintext length to its caller as
info->size_unciphered, without relating the two. size_unciphered comes
from the image's 'data-size-unciphered' property, so an image can claim
a plaintext larger than the buffer that was allocated for it.

fit_image_uncipher() propagates that length as the image size, and
everything downstream - the load, the copy to the entry point - works
from it, reading up to 4 GiB past the end of the decrypted buffer.

Unlike the image data itself, 'data-size-unciphered' is not covered by
the per-image hash or signature, so this is reachable on a signed FIT
whose signature still verifies.

Decryption produces exactly cipher_len bytes, so require the claimed
size to fit within that.

Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
 lib/aes/aes-decrypt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/aes/aes-decrypt.c b/lib/aes/aes-decrypt.c
index 85773a9c4f6..5d616983082 100644
--- a/lib/aes/aes-decrypt.c
+++ b/lib/aes/aes-decrypt.c
@@ -27,6 +27,15 @@ int image_aes_decrypt(struct image_cipher_info *info,
 		return -EINVAL;
 	}
 
+	/*
+	 * Decryption produces exactly cipher_len bytes, so the unciphered
+	 * size the image claims cannot be larger than that.
+	 */
+	if (info->size_unciphered > cipher_len) {
+		printf("Invalid unciphered size\n");
+		return -EINVAL;
+	}
+
 	*data = malloc(cipher_len);
 	if (!*data) {
 		printf("Can't allocate memory to decrypt\n");
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v1 1/3] lib: aes: reject a ciphertext length that is not a whole number of blocks

[Simon Glass]

On 2026-08-15T22:07:52, Pranav Rajendran <pranavkasthuri@gmail.com> wrote:
> lib: aes: reject a ciphertext length that is not a whole number of blocks
>
> image_aes_decrypt() allocates cipher_len bytes for the plaintext but
> then asks aes_cbc_decrypt_blocks() to write
> DIV_ROUND_UP(cipher_len, AES_BLOCK_LENGTH) blocks into it. For a
> cipher_len that is not a multiple of AES_BLOCK_LENGTH the rounding up
> adds one block, so the last block is written up to AES_BLOCK_LENGTH - 1
> bytes past the end of the allocation, and read the same distance past
> the end of the ciphertext.
>
> cipher_len is the size of the image data in the FIT, so an image with a
> 'data' property whose length is not block aligned is enough to reach
> this. The overflowing bytes are decryption output, i.e. they depend on
> the key, but the length itself is not covered by anything that would
> stop the image from being parsed this far.
>
> A CBC ciphertext is a whole number of blocks by construction, so treat
> anything else as a malformed image and reject it before allocating.
> With that established, compute the block count with a plain division so
> the buffer size and the write length cannot drift apart again.
>
> Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
> Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
>
> lib/aes/aes-decrypt.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v1 2/3] image-fit: check the length of the data-size-unciphered property

[Simon Glass]

Hi Pranav,

On 2026-08-15T22:07:52, Pranav Rajendran <pranavkasthuri@gmail.com> wrote:
> image-fit: check the length of the data-size-unciphered property
>
> fit_image_get_data_size_unciphered() passes NULL as fdt_getprop()'s
> length argument, so it accepts a 'data-size-unciphered' property of any
> size and then dereferences the first four bytes of it. A property
> shorter than that is read past its end, and the bytes that follow it in
> the FIT are returned to the caller as the unciphered size.
>
> Ask for the length and require it to be exactly one fdt32_t, as the
> binding describes.
>
> Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
> Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
>
> boot/image-fit.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

> diff --git a/boot/image-fit.c b/boot/image-fit.c
> @@ -1040,11 +1040,15 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
>                                      size_t *data_size)
>  {
>       const fdt32_t *val;
> +     int len;
>
> -     val = fdt_getprop(fit, noffset, "data-size-unciphered", NULL);
> +     val = fdt_getprop(fit, noffset, "data-size-unciphered", &len);
>       if (!val)
>               return -ENOENT;
>
> +     if (len != sizeof(*val))
> +             return -EINVAL;
> +
>       *data_size = (size_t)fdt32_to_cpu(*val);

Please add -EINVAL to the kernel-doc block above so the contract
matches the code. Otherwise this looks right.

With that:

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

Re: [PATCH v1 3/3] lib: aes: reject an unciphered size larger than the ciphertext

[Simon Glass]

On 2026-08-15T22:07:52, Pranav Rajendran <pranavkasthuri@gmail.com> wrote:
> lib: aes: reject an unciphered size larger than the ciphertext
>
> image_aes_decrypt() allocates cipher_len bytes, decrypts into them, and
> then reports the plaintext length to its caller as
> info->size_unciphered, without relating the two. size_unciphered comes
> from the image's 'data-size-unciphered' property, so an image can claim
> a plaintext larger than the buffer that was allocated for it.
>
> fit_image_uncipher() propagates that length as the image size, and
> everything downstream - the load, the copy to the entry point - works
> from it, reading up to 4 GiB past the end of the decrypted buffer.
>
> Unlike the image data itself, 'data-size-unciphered' is not covered by
> the per-image hash or signature, so this is reachable on a signed FIT
> whose signature still verifies.
>
> Decryption produces exactly cipher_len bytes, so require the claimed
> size to fit within that.
>
> Fixes: 4df3578119b0 ("u-boot: fit: add support to decrypt fit with aes")
> Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
>
> lib/aes/aes-decrypt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [v1,0/3] fit: cipher: bounds checks on the ciphered image path

[Simon Glass]

Hi Pranav,

On 2026-08-15T22:07:52, Pranav Rajendran <pranavkasthuri@gmail.com> wrote:

> I have kept them as separate patches since they are independent defects
> with different consequences, but I am happy to squash 1 and 3 if you
> would rather have one change per function.

Please keep them separate - distinct Fixes: targets, and easier to
back-port independently.

Since these are security fixes, how about adding regression tests?
test/lib/test_aes.c covers the primitive, but nothing exercises
image_aes_decrypt() or fit_image_get_data_size_unciphered() against a
malformed FIT. A small sandbox test constructing a FIT with a
non-block-aligned 'data', a truncated 'data-size-unciphered', and an
oversized unciphered size would pin all three down. What do you think?

Regards,
Simon