Re: [PATCH nf 0/1] netfilter: h323: fix helper NAT mangling

On Wed, Jul 29, 2026 at 3:30 PM Florian Westphal <[email protected]> wrote:
>
> zhilin zou <[email protected]> wrote:
> > > > packet's current IPv4 protocol byte. A namespace-local packet modifier
> > > > can rewrite that byte after conntrack has accepted the original layout
> > > > and before helper processing at the confirm hook.
>
> But it can mangle packet in other ways, no?
>
> AFAICS this fix papers over the problem and only "fixes" this particular reproducer.
>
> > > commit 54f34607d184c1cc056c59a5b3d86d96dd6a515c
> > > Author: Florian Westphal <[email protected]>
> > > Date:   Tue Jun 9 13:51:53 2026 +0200
> > >
> > >     netfilter: nfnetlink_queue: restrict writes to network header
> > >
> > > commit df07998dfd40796a05fff7ffea2661ad65ed42a7
> > > Author: Florian Westphal <[email protected]>
> > > Date:   Tue Jun 9 13:51:54 2026 +0200
> > >
> > >     netfilter: nftables: restrict linklayer and network header writes
> > >
> > > are you running a kernel with this patches?
>
> Neither fixes are sufficient, afaics, as no tcp revalidation is done.
>
> Ideally we could just remove nfqueue write support, but we can't.
>
> > The issue still reproduces. The crash is:
> >
> > BUG: KASAN: out-of-bounds in mangle_contents+0x13a/0x680
> >
> > and the trace goes through:
> >
> > mangle_contents()
> > __nf_nat_mangle_tcp_packet()
> > set_addr()
> > set_ras_addr()
> > ras_help()
> > nfqnl_recv_verdict()
> >
> > So these commits do not seem to block this NFQUEUE/H.323 NAT mangling path.
>
> Why do you think your patches block this path?
>
> Can't you make a valid TCP packet, then mangle it so that tcp options
> will get you into end-of-buffer territory?
>
> AFAICS you will need to ask your AI to extend the two commits, for nfqueue,
> add nfqnl_validate_l4(), called from the tail of ip option validation, for
> ipv6, called from ipv6 exthdr validation, with the last "next" header as
> argument.
>
> - Validate minimal size of base header
> - validate length in case of IPPROTO_TCP (data_len >= sizeof(struct tcphdr)
>   and data_len >= __tcp_hdrlen())
> - reject unknown headers
> - reject if l4 proto is different from attached
>   nf_conn l4proto (if any).
>
> For nft_payload.c: add nft_th_write_ok(), which
> rejects writes to TCP th->doff.
>
> nft_nh_write() rejects writes to nexthdr value, so "nf_conn l4 proto
> changes underneath" should not be possible via nft_payload.

Hi Florian,

Thanks for the detailed feedback.

I agree that the current H.323-specific change only addresses the
mismatch used by this reproducer, and does not cover the more general
case where NFQUEUE can return a packet whose L4 header is no longer
consistent with the conntrack state or with its own header lengths.

I will rework this for v2 along the lines you suggested: add L4
validation after NFQUEUE packet modification/reinjection, including TCP
minimum header and data-offset checks, reject packets whose L4 protocol
no longer matches an attached nf_conn where applicable, and update
nft_payload.c to reject writes to TCP doff.

Thanks,
Zhiling