bugzilla-daemon at netfilter.org
2023-Jul-03 07:31 UTC
[Bug 1691] New: mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues
https://bugzilla.netfilter.org/show_bug.cgi?id=1691 Bug ID: 1691 Summary: mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues Product: libmnl Version: unspecified Hardware: x86_64 OS: All Status: NEW Severity: normal Priority: P5 Component: libmnl Assignee: pablo at netfilter.org Reporter: eliogovea1993 at gmail.com Consider the following example: source: ``` #include <linux/netfilter.h> #include <assert.h> #include <stdio.h> #include <stdlib.h> #include <libmnl/libmnl.h> int main(int argc, char** argv) { const struct nlmsghdr header = { .nlmsg_len = 0xFFFFFFFF, .nlmsg_type = 0x0000, .nlmsg_flags = 0x0000, .nlmsg_seq = 0x00000000, .nlmsg_pid = 0x00000000 }; assert(!mnl_nlmsg_ok(&header, sizeof(struct nlmsghdr))); return EXIT_SUCCESS; } ``` output: ``` parse-header: parse-header.c:19: main: Assertion `!mnl_nlmsg_ok(&header, sizeof(struct nlmsghdr))' failed. Aborted (core dumped) ``` The previous example has a malformed message, the "buffer" used as argument for "mnl_nlmsg_ok" doesn't hold a complete message (length "0xFFFFFFFF" is higher than sizeof(header)), yet the function returns true. It seems unlikely that a message will have that "nlmsg_len" in the header, but this report assumes that such bytes are possible while parsing, and a parser using libmnl must correctly identify any incomplete messages. Looking at the source code ``` EXPORT_SYMBOL bool mnl_nlmsg_ok(const struct nlmsghdr *nlh, int len) { return len >= (int)sizeof(struct nlmsghdr) && nlh->nlmsg_len >= sizeof(struct nlmsghdr) && (int)nlh->nlmsg_len <= len; } ``` The last line casts "nlh->nlmsg_len" to "int", but that value can't be stored in an "int" and casting it makes its value negative. To fix it, consider the following patch: ``` diff --git a/src/nlmsg.c b/src/nlmsg.c index c634501..8d0107f 100644 --- a/src/nlmsg.c +++ b/src/nlmsg.c @@ -154,7 +154,7 @@ EXPORT_SYMBOL bool mnl_nlmsg_ok(const struct nlmsghdr *nlh, int len) { return len >= (int)sizeof(struct nlmsghdr) && nlh->nlmsg_len >= sizeof(struct nlmsghdr) && - (int)nlh->nlmsg_len <= len; + nlh->nlmsg_len <= (unsigned int)len; } /** Casting "len" to "unsigned int" would be correct since the first line already checks that it is positive. Since "mnl_nlmsg_ok" is used in a while loop in "__mnl_cb_run", parsing malformed messages like the previous one will try to dereference a "struct nlmsghdr*" out of the limits of the message buffer. ``` -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230703/aab736ac/attachment.html>
bugzilla-daemon at netfilter.org
2023-Sep-10 20:24 UTC
[Bug 1691] mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues
https://bugzilla.netfilter.org/show_bug.cgi?id=1691 Jeremy Sowden <jeremy at azazel.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jeremy at azazel.net Assignee|pablo at netfilter.org |jeremy at azazel.net --- Comment #1 from Jeremy Sowden <jeremy at azazel.net> --- My preference would be to get rid of the casts altogether: EXPORT_SYMBOL bool mnl_nlmsg_ok(const struct nlmsghdr *nlh, int len) { size_t ulen = len; if (len < 0) return 0; return ulen >= sizeof(struct nlmsghdr) && nlh->nlmsg_len >= sizeof(struct nlmsghdr) && nlh->nlmsg_len <= ulen; } I will send a patch. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230910/67446af2/attachment.html>
bugzilla-daemon at netfilter.org
2024-Mar-28 21:31 UTC
[Bug 1691] mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues
https://bugzilla.netfilter.org/show_bug.cgi?id=1691 Jeremy Sowden <jeremy at azazel.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |FIXED --- Comment #2 from Jeremy Sowden <jeremy at azazel.net> --- http://git.netfilter.org/libmnl/commit/?id=754c9de5ea1bea821495523cf01989299552e524 -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20240328/7fee251b/attachment.html>