Skip to content

Commit 4701afa

Browse files
committed
netfilter: nf_tables: reject QUEUE/DROP verdict parameters
jira LE-831 cve CVE-2024-1086 commit f342de4 This reverts commit e0abdad. core.c:nf_hook_slow assumes that the upper 16 bits of NF_DROP verdicts contain a valid errno, i.e. -EPERM, -EHOSTUNREACH or similar, or 0. Due to the reverted commit, its possible to provide a positive value, e.g. NF_ACCEPT (1), which results in use-after-free. Its not clear to me why this commit was made. NF_QUEUE is not used by nftables; "queue" rules in nftables will result in use of "nft_queue" expression. If we later need to allow specifiying errno values from userspace (do not know why), this has to call NF_DROP_GETERR and check that "err <= 0" holds true. Fixes: e0abdad ("netfilter: nf_tables: accept QUEUE/DROP verdict parameters") Cc: [email protected] Reported-by: Notselwyn <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit f342de4) Signed-off-by: Jonathan Maple <[email protected]>
1 parent 8a0d38c commit 4701afa

File tree

1 file changed

+6
-10
lines changed

1 file changed

+6
-10
lines changed

net/netfilter/nf_tables_api.c

+6-10
Original file line numberDiff line numberDiff line change
@@ -9713,16 +9713,10 @@ static int nft_verdict_init(const struct nft_ctx *ctx, struct nft_data *data,
97139713
data->verdict.code = ntohl(nla_get_be32(tb[NFTA_VERDICT_CODE]));
97149714

97159715
switch (data->verdict.code) {
9716-
default:
9717-
switch (data->verdict.code & NF_VERDICT_MASK) {
9718-
case NF_ACCEPT:
9719-
case NF_DROP:
9720-
case NF_QUEUE:
9721-
break;
9722-
default:
9723-
return -EINVAL;
9724-
}
9725-
fallthrough;
9716+
case NF_ACCEPT:
9717+
case NF_DROP:
9718+
case NF_QUEUE:
9719+
break;
97269720
case NFT_CONTINUE:
97279721
case NFT_BREAK:
97289722
case NFT_RETURN:
@@ -9756,6 +9750,8 @@ static int nft_verdict_init(const struct nft_ctx *ctx, struct nft_data *data,
97569750
chain->use++;
97579751
data->verdict.chain = chain;
97589752
break;
9753+
default:
9754+
return -EINVAL;
97599755
}
97609756

97619757
desc->len = sizeof(data->verdict);

0 commit comments

Comments
 (0)