[PATCH nf] netfilter: nat: zero newrange before filling it in

[PATCH nf] netfilter: nat: zero newrange before filling it in

[Florian Westphal]

nf_nat_masquerade_ipv4(), nf_nat_masquerade_ipv6(), netmap_tg4() and
netmap_tg6() each build a stack-local struct nf_nat_range2 from the
caller-supplied range, inheriting range->flags unmodified. Two of them
only memset() the min_addr/max_addr members, the other two don't
memset() anything at all, so newrange.base_proto is left as stack
garbage.

If the inherited flags include NF_NAT_RANGE_PROTO_OFFSET,
nf_nat_l4proto_unique_tuple() reads range->base_proto.all to compute
the NAT port offset. Neither the NETMAP nor the MASQUERADE targets
mask the flags field in their checkentry callback, so this
uninitialized stack value can end up feeding the NAT port selection.

The per-member memset()s are not enough: they have to enumerate every
field that must be cleared, and base_proto was missed when
NF_NAT_RANGE_PROTO_OFFSET/base_proto was added to the struct. memset()
the whole struct instead, so every member is covered regardless of
which ones get explicitly assigned afterwards.

There are other places that miss this, but those hardcode .flags so
nat core won't read from the uninitialized fields.

C99 initializers would be preferrable, but this is a more intrusive
change.  Lets followup in nf-next and convert others as well.

This is an LLM finding.

Fixes: 2eb0f624b709 ("netfilter: add NAT support for shifted portmap ranges")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_nat_masquerade.c | 4 ++--
 net/netfilter/xt_NETMAP.c         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_nat_masquerade.c b/net/netfilter/nf_nat_masquerade.c
index 4de6e0a51701..c2a85bd84af1 100644
--- a/net/netfilter/nf_nat_masquerade.c
+++ b/net/netfilter/nf_nat_masquerade.c
@@ -62,8 +62,7 @@ nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
 		nat->masq_index = out->ifindex;
 
 	/* Transfer from original range. */
-	memset(&newrange.min_addr, 0, sizeof(newrange.min_addr));
-	memset(&newrange.max_addr, 0, sizeof(newrange.max_addr));
+	memset(&newrange, 0, sizeof(newrange));
 	newrange.flags       = range->flags | NF_NAT_RANGE_MAP_IPS;
 	newrange.min_addr.ip = newsrc;
 	newrange.max_addr.ip = newsrc;
@@ -242,6 +241,7 @@ nf_nat_masquerade_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range,
 	if (nat)
 		nat->masq_index = out->ifindex;
 
+	memset(&newrange, 0, sizeof(newrange));
 	newrange.flags		= range->flags | NF_NAT_RANGE_MAP_IPS;
 	newrange.min_addr.in6	= src;
 	newrange.max_addr.in6	= src;
diff --git a/net/netfilter/xt_NETMAP.c b/net/netfilter/xt_NETMAP.c
index 7da065a472e0..8bcb3f716d68 100644
--- a/net/netfilter/xt_NETMAP.c
+++ b/net/netfilter/xt_NETMAP.c
@@ -44,6 +44,7 @@ netmap_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 				   netmask.ip6[i];
 	}
 
+	memset(&newrange, 0, sizeof(newrange));
 	newrange.flags	= range->flags | NF_NAT_RANGE_MAP_IPS;
 	newrange.min_addr	= new_addr;
 	newrange.max_addr	= new_addr;
@@ -91,8 +92,7 @@ netmap_tg4(struct sk_buff *skb, const struct xt_action_param *par)
 		new_ip = ip_hdr(skb)->saddr & ~netmask;
 	new_ip |= mr->range[0].min_ip & netmask;
 
-	memset(&newrange.min_addr, 0, sizeof(newrange.min_addr));
-	memset(&newrange.max_addr, 0, sizeof(newrange.max_addr));
+	memset(&newrange, 0, sizeof(newrange));
 	newrange.flags	     = mr->range[0].flags | NF_NAT_RANGE_MAP_IPS;
 	newrange.min_addr.ip = new_ip;
 	newrange.max_addr.ip = new_ip;
-- 
2.55.0

Re: [PATCH nf] netfilter: nat: zero newrange before filling it in

[Fernando Fernandez Mancera]

On 10/29/26 5:54 PM, Florian Westphal wrote:
> nf_nat_masquerade_ipv4(), nf_nat_masquerade_ipv6(), netmap_tg4() and
> netmap_tg6() each build a stack-local struct nf_nat_range2 from the
> caller-supplied range, inheriting range->flags unmodified. Two of them
> only memset() the min_addr/max_addr members, the other two don't
> memset() anything at all, so newrange.base_proto is left as stack
> garbage.
> 
> If the inherited flags include NF_NAT_RANGE_PROTO_OFFSET,
> nf_nat_l4proto_unique_tuple() reads range->base_proto.all to compute
> the NAT port offset. Neither the NETMAP nor the MASQUERADE targets
> mask the flags field in their checkentry callback, so this
> uninitialized stack value can end up feeding the NAT port selection.
> 
> The per-member memset()s are not enough: they have to enumerate every
> field that must be cleared, and base_proto was missed when
> NF_NAT_RANGE_PROTO_OFFSET/base_proto was added to the struct. memset()
> the whole struct instead, so every member is covered regardless of
> which ones get explicitly assigned afterwards.
> 
> There are other places that miss this, but those hardcode .flags so
> nat core won't read from the uninitialized fields.
> 
> C99 initializers would be preferrable, but this is a more intrusive
> change.  Lets followup in nf-next and convert others as well.
> 
> This is an LLM finding.
> 
> Fixes: 2eb0f624b709 ("netfilter: add NAT support for shifted portmap ranges")
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

I agree that using C99 struct initializers would be preferrable but 
let's look into that later.

Thanks Florian!

Re: [PATCH nf] netfilter: nat: zero newrange before filling it in

[Florian Westphal]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Thanks for double-checking!

> I agree that using C99 struct initializers would be preferrable but let's
> look into that later.

FTR, I do have those patches already; I will send them out once this
patch appears in net-next (via nf -> net -> net-next chain).