<html>
<head>
<base href="https://bugzilla.netfilter.org/" />
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues"
href="https://bugzilla.netfilter.org/show_bug.cgi?id=1691">1691</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>mnl_nlmsg_ok returns true on malformed/incomplete messages leading to potential runtime issues
</td>
</tr>
<tr>
<th>Product</th>
<td>libmnl
</td>
</tr>
<tr>
<th>Version</th>
<td>unspecified
</td>
</tr>
<tr>
<th>Hardware</th>
<td>x86_64
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Priority</th>
<td>P5
</td>
</tr>
<tr>
<th>Component</th>
<td>libmnl
</td>
</tr>
<tr>
<th>Assignee</th>
<td>pablo@netfilter.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>eliogovea1993@gmail.com
</td>
</tr></table>
<p>
<div>
<pre>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.
```</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are watching all bug changes.</li>
</ul>
</body>
</html>