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

[Petr Štetiar]

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

Hi,

> @@ -35,10 +35,16 @@ static bool blobmsg_check_name(const struct blob_attr *attr, size_t len, bool na
>  	char *limit = (char *) attr + len;
>  	const struct blobmsg_hdr *hdr;
>  
> +	if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr))
> +		return false;
> +

why is this change needed? If I'm reading it correctly, then blobmsg_check_name
is only called from blobmsg_check_attr_len and there is already much stricter guard:

 bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t len)
 {
        if (len < sizeof(struct blob_attr))
                return false;

        if (!blobmsg_check_name(attr, len, name))
                return false;

>  	hdr = blob_data(attr);
>  	if (name && !hdr->namelen)
>  		return false;
>  
> +	if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr) + blobmsg_namelen(hdr) + 1)
> +		return false;

Didn't checked it in detail yet, but isn't it almost the same as the check few
lines bellow, just written differently?

>  	if ((char *) hdr->name + blobmsg_namelen(hdr) > limit)
>  		return false;

...

> @@ -191,7 +197,11 @@ int blobmsg_parse(const struct blobmsg_policy *policy, int policy_len,
>  	}
>  
>  	__blob_for_each_attr(attr, data, len) {
> +		if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr))
> +			return -1;

If there is such problem, then this should be probably fixed directly in
__blob_for_each_attr so we possibly protect other __blob_for_each_attr
users[1].

>  		hdr = blob_data(attr);
> +		if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr) + blobmsg_namelen(hdr) + 1)
> +			return -1;

It would be really nice to have blobmsg which could prove, that this check is needed.

1. https://lxr.openwrt.org/ident?i=__blob_for_each_attr

-- ynezz

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