[U-Boot] [PATCH 1/2] image-sig: Ensure that hashed-nodes is null-terminated

From: Konrad Beckmann konrad.beckmann@gmail.com
A specially crafted FIT image leads to memory corruption in the stack when using the verified boot feature. The function fit_config_check_sig has a logic error that makes it possible to write past the end of the stack allocated array node_inc. This could potentially be used to bypass the signature check when using verified boot.
This change ensures that the number of strings is correct when counted.
Signed-off-by: Konrad Beckmann konrad.beckmann@gmail.com --- common/image-sig.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/common/image-sig.c b/common/image-sig.c index 5a269d3289bf..5d860e126637 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -334,6 +334,11 @@ int fit_config_check_sig(const void *fit, int noffset, int required_keynode, return -1; }
+ if (prop && prop_len > 0 && prop[prop_len - 1] != '\0') { + *err_msgp = "hashed-nodes property must be null-terminated"; + return -1; + } + /* Add a sanity check here since we are using the stack */ if (count > IMAGE_MAX_HASHED_NODES) { *err_msgp = "Number of hashed nodes exceeds maximum";

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(+)
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 != '/')

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

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!

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 leads to memory corruption in the stack when using the verified boot feature. The function fit_config_check_sig has a logic error that makes it possible to write past the end of the stack allocated array node_inc. This could potentially be used to bypass the signature check when using verified boot.
This change ensures that the number of strings is correct when counted.
Signed-off-by: Konrad Beckmann konrad.beckmann@gmail.com
common/image-sig.c | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Wed, Nov 07, 2018 at 02:51:45PM -0500, Tom Rini wrote:
From: Konrad Beckmann konrad.beckmann@gmail.com
A specially crafted FIT image leads to memory corruption in the stack when using the verified boot feature. The function fit_config_check_sig has a logic error that makes it possible to write past the end of the stack allocated array node_inc. This could potentially be used to bypass the signature check when using verified boot.
This change ensures that the number of strings is correct when counted.
Signed-off-by: Konrad Beckmann konrad.beckmann@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (2)
-
Simon Glass
-
Tom Rini