backport netfilter nf_table sanitiation fixes
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht
parent
4a8e848f62
commit
ad0a766ee2
@ -0,0 +1,103 @@
|
|||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Pablo Neira Ayuso <pablo@netfilter.org>
|
||||||
|
Date: Wed, 25 May 2022 10:36:38 +0200
|
||||||
|
Subject: [PATCH] netfilter: nf_tables: disallow non-stateful expression in
|
||||||
|
sets earlier
|
||||||
|
|
||||||
|
CVE-2022-1966
|
||||||
|
|
||||||
|
Since 3e135cd499bf ("netfilter: nft_dynset: dynamic stateful expression
|
||||||
|
instantiation"), it is possible to attach stateful expressions to set
|
||||||
|
elements.
|
||||||
|
|
||||||
|
cd5125d8f518 ("netfilter: nf_tables: split set destruction in deactivate
|
||||||
|
and destroy phase") introduces conditional destruction on the object to
|
||||||
|
accomodate transaction semantics.
|
||||||
|
|
||||||
|
nft_expr_init() calls expr->ops->init() first, then check for
|
||||||
|
NFT_STATEFUL_EXPR, this stills allows to initialize a non-stateful
|
||||||
|
lookup expressions which points to a set, which might lead to UAF since
|
||||||
|
the set is not properly detached from the set->binding for this case.
|
||||||
|
Anyway, this combination is non-sense from nf_tables perspective.
|
||||||
|
|
||||||
|
This patch fixes this problem by checking for NFT_STATEFUL_EXPR before
|
||||||
|
expr->ops->init() is called.
|
||||||
|
|
||||||
|
The reporter provides a KASAN splat and a poc reproducer (similar to
|
||||||
|
those autogenerated by syzbot to report use-after-free errors). It is
|
||||||
|
unknown to me if they are using syzbot or if they use similar automated
|
||||||
|
tool to locate the bug that they are reporting.
|
||||||
|
|
||||||
|
For the record, this is the KASAN splat.
|
||||||
|
|
||||||
|
[ 85.431824] ==================================================================
|
||||||
|
[ 85.432901] BUG: KASAN: use-after-free in nf_tables_bind_set+0x81b/0xa20
|
||||||
|
[ 85.433825] Write of size 8 at addr ffff8880286f0e98 by task poc/776
|
||||||
|
[ 85.434756]
|
||||||
|
[ 85.434999] CPU: 1 PID: 776 Comm: poc Tainted: G W 5.18.0+ #2
|
||||||
|
[ 85.436023] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
|
||||||
|
|
||||||
|
Fixes: 0b2d8a7b638b ("netfilter: nf_tables: add helper functions for expression handling")
|
||||||
|
Reported-and-tested-by: Aaron Adams <edg-e@nccgroup.com>
|
||||||
|
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
||||||
|
(cherry picked from commit 520778042ccca019f3ffa136dd0ca565c486cedd net.git)
|
||||||
|
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
|
||||||
|
Acked-by: Andrea Righi <andrea.righi@canonical.com>
|
||||||
|
Acked-by: Stefan Bader <stefan.bader@canonical.com>
|
||||||
|
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
|
||||||
|
---
|
||||||
|
net/netfilter/nf_tables_api.c | 19 ++++++++++---------
|
||||||
|
1 file changed, 10 insertions(+), 9 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
|
||||||
|
index 2feb88ffcd81..91865dc2bcbb 100644
|
||||||
|
--- a/net/netfilter/nf_tables_api.c
|
||||||
|
+++ b/net/netfilter/nf_tables_api.c
|
||||||
|
@@ -2778,27 +2778,31 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
|
||||||
|
|
||||||
|
err = nf_tables_expr_parse(ctx, nla, &expr_info);
|
||||||
|
if (err < 0)
|
||||||
|
- goto err1;
|
||||||
|
+ goto err_expr_parse;
|
||||||
|
+
|
||||||
|
+ err = -EOPNOTSUPP;
|
||||||
|
+ if (!(expr_info.ops->type->flags & NFT_EXPR_STATEFUL))
|
||||||
|
+ goto err_expr_stateful;
|
||||||
|
|
||||||
|
err = -ENOMEM;
|
||||||
|
expr = kzalloc(expr_info.ops->size, GFP_KERNEL);
|
||||||
|
if (expr == NULL)
|
||||||
|
- goto err2;
|
||||||
|
+ goto err_expr_stateful;
|
||||||
|
|
||||||
|
err = nf_tables_newexpr(ctx, &expr_info, expr);
|
||||||
|
if (err < 0)
|
||||||
|
- goto err3;
|
||||||
|
+ goto err_expr_new;
|
||||||
|
|
||||||
|
return expr;
|
||||||
|
-err3:
|
||||||
|
+err_expr_new:
|
||||||
|
kfree(expr);
|
||||||
|
-err2:
|
||||||
|
+err_expr_stateful:
|
||||||
|
owner = expr_info.ops->type->owner;
|
||||||
|
if (expr_info.ops->type->release_ops)
|
||||||
|
expr_info.ops->type->release_ops(expr_info.ops);
|
||||||
|
|
||||||
|
module_put(owner);
|
||||||
|
-err1:
|
||||||
|
+err_expr_parse:
|
||||||
|
return ERR_PTR(err);
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -5318,9 +5322,6 @@ struct nft_expr *nft_set_elem_expr_alloc(const struct nft_ctx *ctx,
|
||||||
|
return expr;
|
||||||
|
|
||||||
|
err = -EOPNOTSUPP;
|
||||||
|
- if (!(expr->ops->type->flags & NFT_EXPR_STATEFUL))
|
||||||
|
- goto err_set_elem_expr;
|
||||||
|
-
|
||||||
|
if (expr->ops->type->flags & NFT_EXPR_GC) {
|
||||||
|
if (set->flags & NFT_SET_TIMEOUT)
|
||||||
|
goto err_set_elem_expr;
|
||||||
@ -0,0 +1,78 @@
|
|||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Pablo Neira Ayuso <pablo@netfilter.org>
|
||||||
|
Date: Fri, 27 May 2022 09:56:18 +0200
|
||||||
|
Subject: [PATCH] netfilter: nf_tables: sanitize nft_set_desc_concat_parse()
|
||||||
|
|
||||||
|
BugLink: https://bugs.launchpad.net/bugs/1976363 (netfilter newset OOB write (LP: #1976363))
|
||||||
|
|
||||||
|
Add several sanity checks for nft_set_desc_concat_parse():
|
||||||
|
|
||||||
|
- validate desc->field_count not larger than desc->field_len array.
|
||||||
|
- field length cannot be larger than desc->field_len (ie. U8_MAX)
|
||||||
|
- total length of the concatenation cannot be larger than register array.
|
||||||
|
|
||||||
|
Joint work with Florian Westphal.
|
||||||
|
|
||||||
|
Fixes: f3a2181e16f1 ("netfilter: nf_tables: Support for sets with multiple ranged fields")
|
||||||
|
Reported-by: <zhangziming.zzm@antgroup.com>
|
||||||
|
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
|
||||||
|
Signed-off-by: Florian Westphal <fw@strlen.de>
|
||||||
|
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
||||||
|
(cherry picked from commit fecf31ee395b0295f2d7260aa29946b7605f7c85 net.git)
|
||||||
|
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
|
||||||
|
Acked-by: Andrea Righi <andrea.righi@canonical.com>
|
||||||
|
Acked-by: Stefan Bader <stefan.bader@canonical.com>
|
||||||
|
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
|
||||||
|
---
|
||||||
|
net/netfilter/nf_tables_api.c | 17 +++++++++++++----
|
||||||
|
1 file changed, 13 insertions(+), 4 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
|
||||||
|
index 91865dc2bcbb..ee4edebe6124 100644
|
||||||
|
--- a/net/netfilter/nf_tables_api.c
|
||||||
|
+++ b/net/netfilter/nf_tables_api.c
|
||||||
|
@@ -4151,6 +4151,9 @@ static int nft_set_desc_concat_parse(const struct nlattr *attr,
|
||||||
|
u32 len;
|
||||||
|
int err;
|
||||||
|
|
||||||
|
+ if (desc->field_count >= ARRAY_SIZE(desc->field_len))
|
||||||
|
+ return -E2BIG;
|
||||||
|
+
|
||||||
|
err = nla_parse_nested_deprecated(tb, NFTA_SET_FIELD_MAX, attr,
|
||||||
|
nft_concat_policy, NULL);
|
||||||
|
if (err < 0)
|
||||||
|
@@ -4160,9 +4163,8 @@ static int nft_set_desc_concat_parse(const struct nlattr *attr,
|
||||||
|
return -EINVAL;
|
||||||
|
|
||||||
|
len = ntohl(nla_get_be32(tb[NFTA_SET_FIELD_LEN]));
|
||||||
|
-
|
||||||
|
- if (len * BITS_PER_BYTE / 32 > NFT_REG32_COUNT)
|
||||||
|
- return -E2BIG;
|
||||||
|
+ if (!len || len > U8_MAX)
|
||||||
|
+ return -EINVAL;
|
||||||
|
|
||||||
|
desc->field_len[desc->field_count++] = len;
|
||||||
|
|
||||||
|
@@ -4173,7 +4175,8 @@ static int nft_set_desc_concat(struct nft_set_desc *desc,
|
||||||
|
const struct nlattr *nla)
|
||||||
|
{
|
||||||
|
struct nlattr *attr;
|
||||||
|
- int rem, err;
|
||||||
|
+ u32 num_regs = 0;
|
||||||
|
+ int rem, err, i;
|
||||||
|
|
||||||
|
nla_for_each_nested(attr, nla, rem) {
|
||||||
|
if (nla_type(attr) != NFTA_LIST_ELEM)
|
||||||
|
@@ -4184,6 +4187,12 @@ static int nft_set_desc_concat(struct nft_set_desc *desc,
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
|
||||||
|
+ for (i = 0; i < desc->field_count; i++)
|
||||||
|
+ num_regs += DIV_ROUND_UP(desc->field_len[i], sizeof(u32));
|
||||||
|
+
|
||||||
|
+ if (num_regs > NFT_REG32_COUNT)
|
||||||
|
+ return -E2BIG;
|
||||||
|
+
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
Loading…
Reference in New Issue
Block a user