`mnl_nlmsg_ok` and `mnl_attr_ok` both expect a signed buffer length value, `len`, against which to compare the size of the object expected to fit into the buffer, because they are intended to validate the length and it may be negative in the case of malformed messages. Comparing this signed value against unsigned operands leads to compiler warnings, so the unsigned operands are cast to `int`. Comparing `len` to the size of the structure is fine, because the structures are only a few bytes in size. Comparing it to the length fields of `struct nlmsg` and `struct nlattr`, however, is problematic, since these fields may hold values greater than `INT_MAX`, in which case the casts will yield negative values and result in false positives. Instead, assign `len` to an unsigned local variable, check for negative values first, then use the unsigned local for the other comparisons, and remove the casts. Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1691 Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx> --- src/attr.c | 9 +++++++-- src/nlmsg.c | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/attr.c b/src/attr.c index bc39df4199e7..48e95019d5e8 100644 --- a/src/attr.c +++ b/src/attr.c @@ -97,9 +97,14 @@ EXPORT_SYMBOL void *mnl_attr_get_payload(const struct nlattr *attr) */ EXPORT_SYMBOL bool mnl_attr_ok(const struct nlattr *attr, int len) { - return len >= (int)sizeof(struct nlattr) && + size_t ulen = len; + + if (len < 0) + return 0; + + return ulen >= sizeof(struct nlattr) && attr->nla_len >= sizeof(struct nlattr) && - (int)attr->nla_len <= len; + attr->nla_len <= ulen; } /** diff --git a/src/nlmsg.c b/src/nlmsg.c index c63450174c67..920cad0e0f46 100644 --- a/src/nlmsg.c +++ b/src/nlmsg.c @@ -152,9 +152,14 @@ EXPORT_SYMBOL void *mnl_nlmsg_get_payload_offset(const struct nlmsghdr *nlh, */ EXPORT_SYMBOL bool mnl_nlmsg_ok(const struct nlmsghdr *nlh, int len) { - return len >= (int)sizeof(struct nlmsghdr) && + size_t ulen = len; + + if (len < 0) + return 0; + + return ulen >= sizeof(struct nlmsghdr) && nlh->nlmsg_len >= sizeof(struct nlmsghdr) && - (int)nlh->nlmsg_len <= len; + nlh->nlmsg_len <= ulen; } /** -- 2.40.1
