Commit 3027ecbd authored by Jiayuan Chen's avatar Jiayuan Chen Committed by Pablo Neira Ayuso
Browse files

netfilter: nft_ct: bail out on template ct in get eval 

I noticed this issue while looking at a historic syzbot report [1].

A rule like the one below is enough to trigger the bug:

    table ip t {
        chain pre {
            type filter hook prerouting priority raw;
            ct zone set 1
            ct original saddr 1.2.3.4 accept
        }
    }

The first expression attaches a per-cpu template ct via
nft_ct_set_zone_eval() (nf_ct_tmpl_alloc -> kzalloc, tuple is all
zero, nf_ct_l3num(ct) == 0). The next expression then calls
nft_ct_get_eval() on the same skb, treats the template as a real ct
and hits the 16-byte memcpy path. With dreg at NFT_REG32_15 this
overflows past struct nft_regs on the kernel stack; with smaller
dreg values it silently clobbers adjacent registers.

Reject template ct at the eval entry and in nft_ct_get_fast_eval(),
mirroring the check nft_ct_set_eval() already has. Additionally,
bound the address copy in NFT_CT_SRC / NFT_CT_DST by priv->len
instead of by nf_ct_l3num(ct): nf_ct_get_tuple() zeroes the tuple
before pkt_to_tuple() fills in only the protocol-relevant leading
bytes, so the trailing bytes of tuple->{src,dst}.u3.all are
well-defined zero. priv->len is validated at rule load, so the
copy size is now bounded by the destination register rather than
by an untrusted field on the conntrack.

[1]: https://syzkaller.appspot.com/bug?id=389cf09cb72926114fce90dc85a2c3231dcb647c



Fixes: 45d9bcda ("netfilter: nf_tables: validate len in nft_validate_data_load()")
Suggested-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarJiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent c32b26aa

net/netfilter/nft_ct.c

+3 −5
Original line number Diff line number Diff line
@@ -78,7 +78,7 @@ static void nft_ct_get_eval(const struct nft_expr *expr, 
		break;

	}



	if (ct == NULL)

	if (!ct || nf_ct_is_template(ct))

		goto err;



	switch (priv->key) {
@@ -180,12 +180,10 @@ static void nft_ct_get_eval(const struct nft_expr *expr, 
	tuple = &ct->tuplehash[priv->dir].tuple;

	switch (priv->key) {

	case NFT_CT_SRC:

		memcpy(dest, tuple->src.u3.all,

		       nf_ct_l3num(ct) == NFPROTO_IPV4 ? 4 : 16);

		memcpy(dest, tuple->src.u3.all, priv->len);

		return;

	case NFT_CT_DST:

		memcpy(dest, tuple->dst.u3.all,

		       nf_ct_l3num(ct) == NFPROTO_IPV4 ? 4 : 16);

		memcpy(dest, tuple->dst.u3.all, priv->len);

		return;

	case NFT_CT_PROTO_SRC:

		nft_reg_store16(dest, (__force u16)tuple->src.u.all);

net/netfilter/nft_ct_fast.c

+1 −1
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ void nft_ct_get_fast_eval(const struct nft_expr *expr, 
		break;

	}



	if (!ct) {

	if (!ct || nf_ct_is_template(ct)) {

		regs->verdict.code = NFT_BREAK;

		return;

	}