On Wed, Oct 11, 2023 at 06:24:27PM +0100, Pierre-Clément Tosi wrote: > As 'offset' is obtained through various paths within the function by > adding user-provided values to 'startoffset' and as we validate its > final value by substracting it from the initial one, there is a risk > that one of the paths might have lead to an overflow, making the check > validate a "negative" (wrong) length, potentially causing fdt_next_tag() > to report an invalid offset as valid through 'nextoffset'. > > For example, when parsing an FDT_PROP, we currently validate that > > offset = startoffset + len + FDT_TAGSIZE > > doesn't overflow but then assign > > offset = startoffset + len + sizeof(struct fdt_property) > > so harden all paths by validating the offset in the very last check. > > Signed-off-by: Pierre-Clément Tosi <ptosi@xxxxxxxxxx> The intended purpose of fdt_offset_ptr() is that you can give it otherwise unvetted values, and it will only return non-NULL if those correspond to a sane range of bytes within the dtb. If the test you're adding here is necessary, then fdt_offset_ptr() isn't accomplishing that job. So, it seems like it's fdt_offset_ptr() that should be hardened instead, which may fix other places as well as this one. > --- > libfdt/fdt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libfdt/fdt.c b/libfdt/fdt.c > index 20c6415..dda342d 100644 > --- a/libfdt/fdt.c > +++ b/libfdt/fdt.c > @@ -213,7 +213,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) > return FDT_END; > } > > - if (!fdt_offset_ptr(fdt, startoffset, offset - startoffset)) > + if (!can_assume(VALID_DTB) && (offset <= startoffset > + || !fdt_offset_ptr(fdt, startoffset, offset - startoffset))) > return FDT_END; /* premature end */ > > *nextoffset = FDT_TAGALIGN(offset); -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
