Since fdt_next_tag() in a public API function all input parameters,
including the fdt blob should not be trusted. It is possible to forge
a blob with invalid property length that will cause integer overflow
during offset calculation. To prevent that, validate the property length
read from the blob before doing calculations.
Signed-off-by: Tadeusz Struk <tadeusz.struk@xxxxxxxxxx>
---
v2:
* Use len local variable to avoid multiple calls to fdt32_to_cpu(*lenp)
* Add can_assume(VALID_DTB) to the new checks
v3:
* Use unsigned integer for prop len and offset validation
---
libfdt/fdt.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index 90a39e8..20c6415 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -162,7 +162,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
{
const fdt32_t *tagp, *lenp;
- uint32_t tag;
+ uint32_t tag, len, sum;
int offset = startoffset;
const char *p;
@@ -188,12 +188,19 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
if (!can_assume(VALID_DTB) && !lenp)
return FDT_END; /* premature end */
+
+ len = fdt32_to_cpu(*lenp);
+ sum = len + offset;
+ if (!can_assume(VALID_DTB) &&
+ (INT_MAX <= sum || sum < (uint32_t) offset))
+ return FDT_END; /* premature end */
+
/* skip-name offset, length and value */
- offset += sizeof(struct fdt_property) - FDT_TAGSIZE
- + fdt32_to_cpu(*lenp);
+ offset += sizeof(struct fdt_property) - FDT_TAGSIZE + len;
+
if (!can_assume(LATEST) &&
- fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 &&
- ((offset - fdt32_to_cpu(*lenp)) % 8) != 0)
+ fdt_version(fdt) < 0x10 && len >= 8 &&
+ ((offset - len) % 8) != 0)
offset += 4;
break;
--
2.37.3
