Re: dtc: 5 memory-safety defect(s) — reports, PoCs and patches attached

[David Gibson <david@gibson.dropbear.id.au>]

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

On Tue, Oct 13, 2026 at 06:15:55PM +0800, 胡景浩  wrote:
> Hello,
> 
> 
> I found 5 defect(s) in dtc. Each has a minimal PoC, a reproduction
> command and a proposed patch.

I'm now looking at these in more detail.

> 1. Negative size passed to memcpy while reading a flattened device tree property
>    CWE-787 / CWE-190 — Medium | CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H (7.8 High)
>    A negative length (derived from the property header) bypasses the bounds check and is passed to memcpy, producing a huge copy.

This has since been fixed by 5664b4514077 ("flattree: reject property
lengths exceeding remaining blob size").


> 2. strchr over-read on a non-terminated property value (alias path checking)
>    CWE-125 — Low | CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:N (5.5 Medium)
>    A property value that is not NUL-terminated is passed to strchr() while checking alias paths, reading past the property buffer.

The proposed patch is nonsense.  It checks whether the path parameter
is NULL (pointer), which is unrelated to whether it points to a NUL
(\0) terminated string.

The "proof of concept" is also nonsense.  The change made from
orig.dts to poc.dts does not remove a terminating \0 from the string.
There is already an existing test that *does* test this case (the
"nonull" property).

Maybe there's a real bug in the vicinity, but please don't waste my
time with garbage reports like this.

> 3. fdtdump reads the memory-reserve map at an unvalidated offset
>    CWE-125 — heap-buffer-overflow read -> crash | CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:N (5.5 Medium)
>    fdtdump computes the reserve-map pointer straight from the header field off_mem_rsvmap and then walks it without ever comparing against totalsize.

Real bug, though the impact for a bug in fdtdump is pretty small.  The
provided fix was not very good.  I've written a better fix which I
will push soon.

> 4. fdtdump passes an unvalidated nameoff to printf("%s")
>    CWE-125 — heap-buffer-overflow read via printf %s -> information leak and crash | CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:N (5.5 Medium)
>    The property-name offset nameoff from the structure block is added to the string-block pointer without any validation against size_dt_strings or totalsize, and the result is handed to printf("%s").

Like 3, a real bug, but anything in fdtump is low impact.  Fix
provided was poor, I've written an improved one which I will push
shortly.

> 5. fdtdump walks the structure block without a totalsize bound
>    CWE-125 — heap-buffer-overflow read | CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:N (5.5 Medium)
>    dump_blob() starts at p_struct and follows tags without bounding the cursor by totalsize / size_dt_struct; strlen() on a node name is likewise unbounded.

Again, real bug, small impact, poor patch.  I've made a much more
robust fix and will push shortly.

-- 
David Gibson (he or they)	| 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: 833 bytes --]