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: 45d9bcda21f4 ("netfilter: nf_tables: validate len in nft_validate_data_load()")
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index fa2cc556331cf569e7d74dde98b1b51e1f380f74..357513c6dcea08d91c4b498af6589b22a398ab5d 100644 (file)
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -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);

diff --git a/net/netfilter/nft_ct_fast.c b/net/netfilter/nft_ct_fast.c
index e684c8a9184877fe16be47e0aacc89056a41982c..ecf7b3a404be261846b244377921e6c2f3735f7c 100644 (file)
--- a/net/netfilter/nft_ct_fast.c
+++ b/net/netfilter/nft_ct_fast.c
@@ -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;
        }