iptables: fix null dereference parsing bitwise operations

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>

diff --git a/iptables/nft-ruleparse.c b/iptables/nft-ruleparse.c
index 757d3c29fc8166f2f14e320f80ae9445f4c07a24..26a605cf401f4e20cf895201da97a482df1cb49d 100644 (file)
--- 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 220bd56dcf9ad60a66da9ad149dda4e75f440f3e..da008070e3016eac2d0498bd406fd62aa7353bf1 100644 (file)
--- 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;
 }