juraj.vijt...@sartura.hr <juraj.vijt...@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@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
