[PATCH nf v2] netfilter: synproxy: check TCP header before verifying checksum

[Palla Raghunath]

synproxy_tg6() calls nf_ip6_checksum() with par->thoff before it checks
that there is actually a TCP header at that offset.

par->thoff comes from ipv6_find_hdr(), which ip6_packet_match() calls
with target -1. ipv6_find_hdr() only makes sure the first two bytes of
each extension header are in the skb. It then adds the header's declared
length to the offset and stops at the first non-extension header, so the
offset it returns can be past the end of the packet. For a CHECKSUM_NONE
skb, nf_ip6_checksum() then calls skb_checksum(skb, 0, thoff, 0), and if
thoff is bigger than skb->len we hit the BUG_ON(len) in skb_checksum():

  kernel BUG at net/core/skbuff.c:3606!
  RIP: 0010:skb_checksum+0x8b2/0x8c0
  Call Trace:
   nf_ip6_checksum+0x1bd/0x320 net/netfilter/utils.c:89
   synproxy_tg6+0x1a4/0x6e0 net/ipv6/netfilter/ip6t_SYNPROXY.c:21
   ip6t_do_table+0xd37/0x15b0 net/ipv6/netfilter/ip6_tables.c:366
   ...

syzbot's reproducer sends a 60-byte packet with an AH header followed by
two hop-by-hop headers. The second hop-by-hop header says the next header
is TCP and has hdrlen 167, which gives a thoff of 1400.

nft_synproxy_do_eval() has the same ordering: it calls nf_checksum()
before it fetches the TCP header.

In both places, get the TCP header with skb_header_pointer() first and
only then verify the checksum, the same way nf_reject_ip6_tcphdr_get()
does it. Packets that don't have a full TCP header at thoff are now
dropped before we try to checksum them.

Fixes: 4ad362282cb4 ("netfilter: add IPv6 SYNPROXY target")
Fixes: a311a8981727 ("netfilter: nft_synproxy: use the family-aware checksum helper")
Reported-by: syzbot+5a8667f002726fc59f88@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=5a8667f002726fc59f88
Reviewed-by: Fernando Fernandez Mancera <fmancera@xxxxxxx>
Signed-off-by: Palla Raghunath <raghunathpalla.0209@xxxxxxxxx>
---
Thanks Fernando for the review, and for spotting the nft_synproxy case
and giving me the chance to fix it in this patch as well.

v2:
- Also fix nft_synproxy_do_eval() and add its Fixes: tag, as Fernando
  suggested
- Drop the comment in synproxy_tg6()
- Add Fernando's Reviewed-by
- Rebase on v7.3-rc7

Testing: I ran syzbot's reproducer on v7.3-rc7 with this patch applied
and ee319bd3a0e9 ("ipv6: do not let ipv6_find_hdr() return an offset
past the packet end") reverted, since that commit already stops the bad
offset from reaching the target on current trees. The BUG doesn't fire
any more. The nft_synproxy change is only build-tested so far; I haven't
managed to get a packet through an nft synproxy rule with the bad offset
yet.

v1: https://lore.kernel.org/all/20260926204519.43402-1-raghunathpalla.0209@xxxxxxxxx/

 net/ipv6/netfilter/ip6t_SYNPROXY.c |  6 +++---
 net/netfilter/nft_synproxy.c       | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index d51d0c3e5fe9..b3aa4fe67589 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -18,13 +18,13 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 	struct synproxy_options opts = {};
 	struct tcphdr *th, _th;
 
-	if (nf_ip6_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
-		return NF_DROP;
-
 	th = skb_header_pointer(skb, par->thoff, sizeof(_th), &_th);
 	if (th == NULL)
 		return NF_DROP;
 
+	if (nf_ip6_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
+		return NF_DROP;
+
 	if (!synproxy_parse_options(skb, par->thoff, th, &opts))
 		return NF_DROP;
 
diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
index 554a96a000f4..6b290a89886d 100644
--- a/net/netfilter/nft_synproxy.c
+++ b/net/netfilter/nft_synproxy.c
@@ -118,12 +118,6 @@ static void nft_synproxy_do_eval(const struct nft_synproxy *priv,
 		return;
 	}
 
-	if (nf_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP,
-			nft_pf(pkt))) {
-		regs->verdict.code = NF_DROP;
-		return;
-	}
-
 	tcp = skb_header_pointer(skb, thoff,
 				 sizeof(struct tcphdr),
 				 &_tcph);
@@ -132,6 +126,12 @@ static void nft_synproxy_do_eval(const struct nft_synproxy *priv,
 		return;
 	}
 
+	if (nf_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP,
+			nft_pf(pkt))) {
+		regs->verdict.code = NF_DROP;
+		return;
+	}
+
 	if (!synproxy_parse_options(skb, thoff, tcp, &opts)) {
 		regs->verdict.code = NF_DROP;
 		return;
-- 
2.34.1