iptables: fix null dereference parsing bitwise operationsHEADmaster

Iptables binary only understands NFT_BITWISE_MASK_XOR bitwise operation and
assumes its attributes are always present without actually checking, which
leads to a segfault in some cases.

This commit introduces this missing check.

| /**
|  * enum nft_bitwise_ops - nf_tables bitwise operations
|  *
|  * @NFT_BITWISE_MASK_XOR: mask-and-xor operation used to implement NOT, AND, OR
|  *                        and XOR boolean operations
|  * @NFT_BITWISE_LSHIFT: left-shift operation          \
|  * @NFT_BITWISE_RSHIFT: right-shift operation         |
|  * @NFT_BITWISE_AND: and operation                    | These all are affected
|  * @NFT_BITWISE_OR: or operation                      |
|  * @NFT_BITWISE_XOR: xor operation                    /
|  */

From iptables/nft-ruleparse.c:

| static void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
| {
|   [...]
|
|   data = nftnl_expr_get(e, NFTNL_EXPR_BITWISE_XOR, &len); // <-- this attribute may not be present
|
|   if (len > sizeof(dreg->bitwise.xor)) {
|     ctx->errmsg = "bitwise xor too large";
|     return;
|   }
|
|   memcpy(dreg->bitwise.xor, data, len); // <-- zero dereference happens here
|
|   data = nftnl_expr_get(e, NFTNL_EXPR_BITWISE_MASK, &len);
|
|   if (len > sizeof(dreg->bitwise.mask)) {
|   	ctx->errmsg = "bitwise mask too large";
|   	return;
|   }
|
|   memcpy(dreg->bitwise.mask, data, len);
|
|   dreg->bitwise.set = true;
|
| }

The bug can be reproduced by creating a rule like this:

| # newrule.json
| {"chain": "example-chain",
|  "expressions": {"elem": [{"data": {"base": 1,
|                                     "dreg": 1,
|                                     "len": 4,
|                                     "offset": 12},
|                            "name": "payload"},
|                           {"data": {"data": {"value": [255, 255, 255, 0]},
|                                     "dreg": 1,
|                                     "len": 4,
|                                     "op": 3,
|                                     "sreg": 1},
|                            "name": "bitwise"},
|                           {"data": {"data": {"value": [1, 2, 3, 0]},
|                                     "op": 0,
|                                     "sreg": 1},
|                            "name": "cmp"},
|                           {"data": {"data": {"verdict": {"code": 1}},
|                                     "dreg": 0},
|                            "name": "immediate"}]},
|  "nfgen-family": 2,
|  "table": "filter"}

| # newrule.sh
| set -euo pipefail
|
| iptables -N example-chain || true
|
| genid="$(
|   ./tools/net/ynl/pyynl/cli.py --spec Documentation/netlink/specs/nftables.yaml \
|     --do getgen --json "{}" --output-json |
|     jq -r ".id"
| )"
|
| ./tools/net/ynl/pyynl/cli.py --spec Documentation/netlink/specs/nftables.yaml \
|   --multi batch-begin "{\"genid\": $genid, \"res-id\": 10}" \
|   --creat --append --multi newrule "$(cat ./newrule.json)" \
|   --creat --multi batch-end '{}' \
|   --output-json

Signed-off-by: Remy D. Farley <one-d-wide@protonmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>

2 files changed, 14 insertions, 1 deletions

diff --git a/iptables/nft-ruleparse.c b/iptables/nft-ruleparse.c
index 757d3c29..26a605cf 100644
--- a/iptables/nft-ruleparse.c
+++ b/iptables/nft-ruleparse.c
@@ -243,6 +243,11 @@ static void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 	data = nftnl_expr_get(e, NFTNL_EXPR_BITWISE_XOR, &len);
 
+	if (!data) {
+		ctx->errmsg = "missing bitwise xor attribute";
+		return;
+	}
+
 	if (len > sizeof(dreg->bitwise.xor)) {
 		ctx->errmsg = "bitwise xor too large";
 		return;
@@ -252,6 +257,11 @@ static void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 	data = nftnl_expr_get(e, NFTNL_EXPR_BITWISE_MASK, &len);
 
+	if (!data) {
+		ctx->errmsg = "missing bitwise mask attribute";
+		return;
+	}
+
 	if (len > sizeof(dreg->bitwise.mask)) {
 		ctx->errmsg = "bitwise mask too large";
 		return;
diff --git a/iptables/nft.c b/iptables/nft.c
index 220bd56d..da008070 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -4008,7 +4008,6 @@ static const char *supported_exprs[] = {
 	"payload",
 	"meta",
 	"cmp",
-	"bitwise",
 	"counter",
 	"immediate",
 	"lookup",
@@ -4035,6 +4034,10 @@ static int nft_is_expr_compatible(struct nftnl_expr *expr, void *data)
 	    nftnl_expr_is_set(expr, NFTNL_EXPR_LOG_GROUP))
 		return 0;
 
+	if (!strcmp(name, "bitwise") &&
+	    nftnl_expr_get_u32(expr, NFTNL_EXPR_BITWISE_OP) == NFT_BITWISE_BOOL)
+		return 0;
+
 	return -1;
 }