From: Binarly Vulnerability Research <[email protected]>
The fdt_check_no_at(), located in boot/image-fit.c, is a helper
that guards against libfdt's unit-address ambiguity; it scans the
FIT tree and rejects any node whose name contains an '@':
static int fdt_check_no_at(const void *fit, int parent)
{
const char *name;
int node;
int ret;
name = fdt_get_name(fit, parent, NULL);
if (!name || strchr(name, '@'))
return -EADDRNOTAVAIL;
fdt_for_each_subnode(node, fit, parent) {
ret = fdt_check_no_at(fit, node);
if (ret)
return ret;
}
return 0;
}
The function is recursive: for every subnode, it calls itself, and
its termination condition is "no more subnodes".
It is called from the fit_check_format() (boot/image-fit.c):
int fit_check_format(const void *fit, ulong size)
{
int ret;
/* A FIT image must be a valid FDT */
ret = fdt_check_header(fit);
if (ret) {
log_debug("Wrong FIT format: not a flattened device
tree (err=%d)\n",
ret);
return -ENOEXEC;
}
if (CONFIG_IS_ENABLED(FIT_FULL_CHECK)) {
/*
* If we are not given the size, make do with
calculating it.
* This is not as secure, so we should consider a flag to
* control this.
*/
if (size == IMAGE_SIZE_INVAL)
size = fdt_totalsize(fit);
ret = fdt_check_full(fit, size);
if (ret)
ret = -EINVAL;
/*
* U-Boot stopped using unit addressed in 2017. Since
libfdt
* can match nodes ignoring any unit address, signature
* verification can see the wrong node if one is
inserted with
* the same name as a valid node but with a unit address
* attached. Protect against this by disallowing unit
addresses.
*/
if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
ret = fdt_check_no_at(fit, 0);
...
The only guard executed prior to fdt_check_no_at() that validates
the nesting depth of the FIT tree is in fdt_check_full():
case FDT_BEGIN_NODE:
depth++;
if (depth > INT_MAX)
return -FDT_ERR_BADSTRUCTURE;
It is only checked that the depth does not exceed INT_MAX, which
means it is theoretically possible to create a FIT image with a
depth of 2147483647 (if the device has enough RAM to support it).
Each call to fdt_check_no_at() consumes 16 bytes of stack space
(one 'const char *' and two int local variables, and a return
address), while introducing a nested entry in the FIT requires 12
bytes (FDT_BEGIN_NODE -- 4 bytes, node name -- at least 4 bytes,
and FDT_END_NODE -- 4 bytes). The stack is therefore guaranteed to
be exhausted on any RAM size available.
Fix: Validate the nesting depth of the FIT tree.
Signed-off-by: Binarly Vulnerability Research <[email protected]>
---
boot/image-fit.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c
index b0fcaf6e17f..0e225ecd0c2 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -41,6 +41,7 @@ DECLARE_GLOBAL_DATA_PTR;
#include <bootm.h>
#include <image.h>
#include <bootstage.h>
+#include <fdt_region.h>
#include <upl.h>
#include <u-boot/crc.h>
@@ -1643,20 +1644,24 @@ int fit_image_check_comp(const void *fit, int noffset,
uint8_t comp)
*
* @fit: FIT to check
* @parent: Parent node to check
- * Return: 0 if OK, -EADDRNOTAVAIL is a node has a name containing '@'
+ * @depth: Current recursion depth
+ * Return: 0 if OK, or error value
*/
-static int fdt_check_no_at(const void *fit, int parent)
+static int fdt_check_no_at(const void *fit, int parent, int depth)
{
const char *name;
int node;
int ret;
+ if (depth >= FDT_MAX_DEPTH)
+ return -FDT_ERR_BADSTRUCTURE;
+
name = fdt_get_name(fit, parent, NULL);
if (!name || strchr(name, '@'))
return -EADDRNOTAVAIL;
fdt_for_each_subnode(node, fit, parent) {
- ret = fdt_check_no_at(fit, node);
+ ret = fdt_check_no_at(fit, node, depth + 1);
if (ret)
return ret;
}
@@ -1696,7 +1701,7 @@ int fit_check_format(const void *fit, ulong size)
* attached. Protect against this by disallowing unit addresses.
*/
if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
- ret = fdt_check_no_at(fit, 0);
+ ret = fdt_check_no_at(fit, 0, 0);
if (ret) {
log_debug("FIT check error %d\n", ret);
--
2.53.0
