[OpenWrt-Devel] [PATCH][libubox] blobmsg: blobmsg_parse and blobmsg_parse_array oob read fixes

Petr Štetiar ynezz at true.cz
Sun Jan 12 06:43:07 EST 2020


juraj.vijtiuk at sartura.hr <juraj.vijtiuk at sartura.hr> [2020-01-12 12:26:18]:

Hi,

thanks for the fix.

> Fix out of bounds read in blobmsg_parse and blobmsg_check_name. The
> out of bounds read happens because blob_attr and blobmsg_hdr have
> flexible array members, whose size is 0 in the corresponding sizeofs.
> For example the __blob_for_each_attr macro checks whether rem >=
> sizeof(struct blob_attr). However, what LibFuzzer discovered was,
> if the input data was only 4 bytes, the data would be casted to blob_attr,
> and later on blob_data(attr) would be called even though attr->data was empty.
> The same issue could appear with data larger than 4 bytes, where data
> wasn't empty, but contained only the start of the blobmsg_hdr struct,
> and blobmsg_hdr name was empty. The bugs were discovered by fuzzing
> blobmsg_parse and blobmsg_array_parse with LibFuzzer.

I don't know if you're aware, but there is already some LibFuzzer based
fuzzing in tests/fuzz/test-fuzz.c and the corpus is in tests/fuzz/corpus.
Those checks are run now automatically by CI after each Git push.

It would be nice, if you could share the fuzz input (and mods to test-fuzz.c
if any) leading to this OOB reads, so we can guard against future regression.

BTW this is not mandatory, it's optional, but I'm going to do this anyway as
I'm wondering why current fuzzing didn't catched this issue, so you're going
to save me some time :-)

Thanks!

-- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list