From: Florian Westphal <fw@strlen.de>
To: netfilter-devel@vger.kernel.org
Cc: Florian Westphal <fw@strlen.de>
Subject: [PATCH nft] netlink: fix stack buffer overrun when emitting ranged expressions
Date: Fri, 14 Mar 2025 13:41:49 +0100 [thread overview]
Message-ID: <20250314124159.2131-1-fw@strlen.de> (raw)
Included bogon input generates following Sanitizer splat:
AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7...
WRITE of size 2 at 0x7fffffffcbe4 thread T0
#0 0x0000003a68b8 in __asan_memset (src/nft+0x3a68b8) (BuildId: 3678ff51a5405c77e3e0492b9a985910efee73b8)
#1 0x0000004eb603 in __mpz_export_data src/gmputil.c:108:2
#2 0x0000004eb603 in netlink_export_pad src/netlink.c:256:2
#3 0x0000004eb603 in netlink_gen_range src/netlink.c:471:2
#4 0x0000004ea250 in __netlink_gen_data src/netlink.c:523:10
#5 0x0000004e8ee3 in alloc_nftnl_setelem src/netlink.c:205:3
#6 0x0000004d4541 in mnl_nft_setelem_batch src/mnl.c:1816:11
Problem is that the range end is emitted to the buffer at the *padded*
location (rounded up to next register size), but buffer sizing is
based of the expression length, not the padded length.
Also extend the test script: Capture stderr and if we see
AddressSanitizer warning, make it fail.
Same bug as the one fixed in 600b84631410 ("netlink: fix stack buffer overflow with sub-reg sized prefixes"),
just in a different function.
Apply same fix: no dynamic array + add a length check.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/netlink.c | 11 +++++++----
tests/shell/testcases/bogons/assert_failures | 17 ++++++++++++++++-
...an_stack_buffer_overrun_in_netlink_gen_range | 6 ++++++
3 files changed, 29 insertions(+), 5 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/asan_stack_buffer_overrun_in_netlink_gen_range
diff --git a/src/netlink.c b/src/netlink.c
index 8e6e2066fe2a..52c2d8009b82 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -462,11 +462,14 @@ static void netlink_gen_verdict(const struct expr *expr,
static void netlink_gen_range(const struct expr *expr,
struct nft_data_linearize *nld)
{
- unsigned int len = div_round_up(expr->left->len, BITS_PER_BYTE) * 2;
- unsigned char data[len];
- unsigned int offset = 0;
+ unsigned int len = (netlink_padded_len(expr->left->len) / BITS_PER_BYTE) * 2;
+ unsigned char data[NFT_MAX_EXPR_LEN_BYTES];
+ unsigned int offset;
- memset(data, 0, len);
+ if (len > sizeof(data))
+ BUG("Value export of %u bytes would overflow", len);
+
+ memset(data, 0, sizeof(data));
offset = netlink_export_pad(data, expr->left->value, expr->left);
netlink_export_pad(data + offset, expr->right->value, expr->right);
nft_data_memcpy(nld, data, len);
diff --git a/tests/shell/testcases/bogons/assert_failures b/tests/shell/testcases/bogons/assert_failures
index 79099427c98a..3dee63b3f97b 100755
--- a/tests/shell/testcases/bogons/assert_failures
+++ b/tests/shell/testcases/bogons/assert_failures
@@ -1,12 +1,27 @@
#!/bin/bash
dir=$(dirname $0)/nft-f/
+tmpfile=$(mktemp)
+
+cleanup()
+{
+ rm -f "$tmpfile"
+}
+
+trap cleanup EXIT
for f in $dir/*; do
- $NFT --check -f "$f"
+ echo "Check $f"
+ $NFT --check -f "$f" 2> "$tmpfile"
if [ $? -ne 1 ]; then
echo "Bogus input file $f did not cause expected error code" 1>&2
exit 111
fi
+
+ if grep AddressSanitizer "$tmpfile"; then
+ echo "Address sanitizer splat for $f" 1>&2
+ cat "$tmpfile"
+ exit 111
+ fi
done
diff --git a/tests/shell/testcases/bogons/nft-f/asan_stack_buffer_overrun_in_netlink_gen_range b/tests/shell/testcases/bogons/nft-f/asan_stack_buffer_overrun_in_netlink_gen_range
new file mode 100644
index 000000000000..2f7872e4accd
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/asan_stack_buffer_overrun_in_netlink_gen_range
@@ -0,0 +1,6 @@
+table ip test {
+ chain y {
+ redirect to :tcp dport map { 83 : 80/3, 84 :4 }
+ }
+}
+
--
2.45.3
next reply other threads:[~2025-03-14 12:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-14 12:41 Florian Westphal [this message]
2025-03-18 12:24 ` [PATCH nft] netlink: fix stack buffer overrun when emitting ranged expressions Pablo Neira Ayuso
2025-03-18 13:24 ` Florian Westphal
2025-03-18 15:34 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250314124159.2131-1-fw@strlen.de \
--to=fw@strlen.de \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.