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

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi,

On Fri, Oct 09, 2026 at 05:26:24PM +0100, Palla Raghunath wrote:
> 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.

I think this qualifies as nf-next material, since this is now fixed in
the core through:

   ee319bd3a0e9 ("ipv6: do not let ipv6_find_hdr() return an offset past the packet end")


> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5a8667f002726fc59f88
> Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>
> Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
> ---
> 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@gmail.com/
> 
>  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
>