[U-Boot,2/2] fdt_region: Ensure that depth never goes below -1

Message ID 1541620306-14314-2-git-send-email-trini@konsulko.com
State Accepted
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot,1/2] image-sig: Ensure that hashed-nodes is null-terminated
Related show

Commit Message

Tom Rini Nov. 7, 2018, 7:51 p.m.
From: Konrad Beckmann <konrad.beckmann@gmail.com>

A specially crafted FIT image makes it possible to overflow the stack
with controlled values when using the verified boot feature. Depending
on the memory layout, this could be used to overwrite configuration
variables on the heap and setting them to 0, e.g. disable signature
verification, thus bypassing it.

This change fixes a bug in fdt_find_regions where the fdt structure is
parsed. A lower value than -1 of depth can lead to a buffer underflow
write on the stack.

Signed-off-by: Konrad Beckmann <konrad.beckmann@gmail.com>
---
 lib/libfdt/fdt_region.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Simon Glass Nov. 13, 2018, 7:54 p.m. | #1
On 7 November 2018 at 11:51, Tom Rini <trini@konsulko.com> wrote:
> From: Konrad Beckmann <konrad.beckmann@gmail.com>
>
> A specially crafted FIT image makes it possible to overflow the stack
> with controlled values when using the verified boot feature. Depending
> on the memory layout, this could be used to overwrite configuration
> variables on the heap and setting them to 0, e.g. disable signature
> verification, thus bypassing it.
>
> This change fixes a bug in fdt_find_regions where the fdt structure is
> parsed. A lower value than -1 of depth can lead to a buffer underflow
> write on the stack.
>
> Signed-off-by: Konrad Beckmann <konrad.beckmann@gmail.com>
> ---
>  lib/libfdt/fdt_region.c | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Nov. 17, 2018, 2:08 p.m. | #2
On Wed, Nov 07, 2018 at 02:51:46PM -0500, Tom Rini wrote:

> From: Konrad Beckmann <konrad.beckmann@gmail.com>
> 
> A specially crafted FIT image makes it possible to overflow the stack
> with controlled values when using the verified boot feature. Depending
> on the memory layout, this could be used to overwrite configuration
> variables on the heap and setting them to 0, e.g. disable signature
> verification, thus bypassing it.
> 
> This change fixes a bug in fdt_find_regions where the fdt structure is
> parsed. A lower value than -1 of depth can lead to a buffer underflow
> write on the stack.
> 
> Signed-off-by: Konrad Beckmann <konrad.beckmann@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

Patch

diff --git a/lib/libfdt/fdt_region.c b/lib/libfdt/fdt_region.c
index d3b9a60e994f..7e9fa9272e80 100644
--- a/lib/libfdt/fdt_region.c
+++ b/lib/libfdt/fdt_region.c
@@ -96,6 +96,9 @@  int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
 			break;
 
 		case FDT_END_NODE:
+			/* Depth must never go below -1 */
+			if (depth < 0)
+				return -FDT_ERR_BADSTRUCTURE;
 			include = want;
 			want = stack[depth--];
 			while (end > path && *--end != '/')