Re: [PATCH] Catch unsigned 32bit overflow when parsing flattened device tree offsets

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On Sun, Jan 03, 2016 at 08:43:35AM +1100, Anton Blanchard wrote:
> We have a couple of checks of the form:
> 
>     if (offset+size > totalsize)
>         die();
> 
> We need to check that offset+size doesn't overflow, otherwise the check
> will pass, and we may access past totalsize.
> 
> Found with AFL.
> 
> Signed-off-by: Anton Blanchard <anton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
> 
> I've attached an example device tree, do we want to add binary blobs
> to the test suite?

I've generally avoided it, but I forget exactly why.  Usually I try to
generate the testcases as dts and compile them, but I'm guessing this dtb is
something that shouldn't be possible as good output from dtc.

It would be possible to construct it from test/trees.S, but just
including the binary blob might be simpler.

Certainly I would like to include this testcase into the testsuite,
one way or another.

> diff --git a/flattree.c b/flattree.c
> index bd99fa2..ec14954 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -889,7 +889,7 @@ struct boot_info *dt_from_blob(const char *fname)
>  
>  	if (version >= 3) {
>  		uint32_t size_str = fdt32_to_cpu(fdt->size_dt_strings);
> -		if (off_str+size_str > totalsize)
> +		if ((off_str+size_str < off_str) || (off_str+size_str > totalsize))
>  			die("String table extends past total size\n");
>  		inbuf_init(&strbuf, blob + off_str, blob + off_str + size_str);
>  	} else {
> @@ -898,7 +898,7 @@ struct boot_info *dt_from_blob(const char *fname)
>  
>  	if (version >= 17) {
>  		size_dt = fdt32_to_cpu(fdt->size_dt_struct);
> -		if (off_dt+size_dt > totalsize)
> +		if ((off_dt+size_dt < off_dt) || (off_dt+size_dt > totalsize))
>  			die("Structure block extends past total size\n");
>  	}
>  



-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]