From ad0a766ee2b271b97397ad0e949e7ce492eef697 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 8 Jun 2022 18:02:33 +0200 Subject: [PATCH] backport netfilter nf_table sanitiation fixes Signed-off-by: Thomas Lamprecht --- ...les-disallow-non-stateful-expression.patch | 103 ++++++++++++++++++ ...les-sanitize-nft_set_desc_concat_par.patch | 78 +++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 patches/kernel/0017-netfilter-nf_tables-disallow-non-stateful-expression.patch create mode 100644 patches/kernel/0018-netfilter-nf_tables-sanitize-nft_set_desc_concat_par.patch diff --git a/patches/kernel/0017-netfilter-nf_tables-disallow-non-stateful-expression.patch b/patches/kernel/0017-netfilter-nf_tables-disallow-non-stateful-expression.patch new file mode 100644 index 0000000..08418c1 --- /dev/null +++ b/patches/kernel/0017-netfilter-nf_tables-disallow-non-stateful-expression.patch @@ -0,0 +1,103 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +Signed-off-by: Pablo Neira Ayuso +(cherry picked from commit 520778042ccca019f3ffa136dd0ca565c486cedd net.git) +Signed-off-by: Thadeu Lima de Souza Cascardo +Acked-by: Andrea Righi +Acked-by: Stefan Bader +Signed-off-by: Thomas Lamprecht +--- + 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; diff --git a/patches/kernel/0018-netfilter-nf_tables-sanitize-nft_set_desc_concat_par.patch b/patches/kernel/0018-netfilter-nf_tables-sanitize-nft_set_desc_concat_par.patch new file mode 100644 index 0000000..3c00bfb --- /dev/null +++ b/patches/kernel/0018-netfilter-nf_tables-sanitize-nft_set_desc_concat_par.patch @@ -0,0 +1,78 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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: +Reviewed-by: Stefano Brivio +Signed-off-by: Florian Westphal +Signed-off-by: Pablo Neira Ayuso +(cherry picked from commit fecf31ee395b0295f2d7260aa29946b7605f7c85 net.git) +Signed-off-by: Thadeu Lima de Souza Cascardo +Acked-by: Andrea Righi +Acked-by: Stefan Bader +Signed-off-by: Thomas Lamprecht +--- + 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; + } +