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

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.