dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

A security issue: "totalsize" isn't correctly checked #66

Closed huntcve closed 1 year ago

huntcve commented 2 years ago

The "totalsize" in header is currently checked against "INT_MAX" instead of real length of DTB, this is usually OK for valid DTB. But for "bad" DTB, this is not enough. Say on a hypervisor system, when libfdt is used to parse a malicious DTB from an untrsuted VM(virtual machine), the "totalsize" in the header can be arbitrary value, for example, attacker can set the "totalsize" to be 0xFFFF0000 but the real length of DTB is just 0xF000, what will happen? I guess there will be multiple out-of-bounds access in libfdt since we are checking node offsets against "totalsize" everywhere.
If we do agree libfdt shall consider the use case of parsing invalid DTB, we then need to carefully handle those inputs in DTB file without assuming they are legal.

dgibson commented 2 years ago

I'm not sure what you mean by the "real length" of the dtb. For most of the code the totalsize field defines the true size of the dtb. For places which take a specific buffer (like fdt_open_into and fdt_create) we do validate totalsize against the buffer size. Likewise fdt_check_full can be used to force a complete check, including verifying totalsize against a given buffer size.

dgibson commented 1 year ago

Closing, in the absence of any follow up as to what's actually wanted here.