On Thu, Sep 29, 2022 at 6:55 PM Tadeusz Struk <tadeusz.struk@xxxxxxxxxx> wrote: > > 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 comma: ...that, validate... > read from the blob before doing calculations. > > Signed-off-by: Tadeusz Struk <tadeusz.struk@xxxxxxxxxx> > --- > libfdt/fdt.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libfdt/fdt.c b/libfdt/fdt.c > index 90a39e8..c3e112a 100644 > --- a/libfdt/fdt.c > +++ b/libfdt/fdt.c > @@ -186,11 +186,17 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) > > case FDT_PROP: > lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp)); > - if (!can_assume(VALID_DTB) && !lenp) > + if (!can_assume(VALID_DTB) && > + (!lenp || (INT_MAX <= fdt32_to_cpu(*lenp)))) We now have fdt32_to_cpu(*lenp) 4 times. Stick it in a local var. > return FDT_END; /* premature end */ > + > /* skip-name offset, length and value */ > offset += sizeof(struct fdt_property) - FDT_TAGSIZE > + fdt32_to_cpu(*lenp); > + > + if (offset < 0) Needs a can_assume(VALID_DTB) check. > + return FDT_END; /* premature end */ > + > if (!can_assume(LATEST) && > fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 && > ((offset - fdt32_to_cpu(*lenp)) % 8) != 0) > -- > 2.37.3 >
