Re: [PATCH] iptables: fix null dereference parsing bitwise operations

[Phil Sutter <phil@nwl.cc>]

Hi Remy,

On Mon, Feb 02, 2026 at 10:14:52AM +0000, Remy D. Farley wrote:
> 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>
> ---
>  iptables/nft-ruleparse.c | 5 +++++
>  iptables/nft.c           | 5 ++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/iptables/nft-ruleparse.c b/iptables/nft-ruleparse.c
> index cdf1af4f..1a9084e3 100644
> --- a/iptables/nft-ruleparse.c
> +++ b/iptables/nft-ruleparse.c
> @@ -232,6 +232,11 @@ static void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
>  	const void *data;
>  	uint32_t len;
>  
> +	if (nftnl_expr_get_u32(e, NFTNL_EXPR_BITWISE_OP) != 0 /* empty or MASK_XOR */) {
> +		ctx->errmsg = "unsupported bitwise operation";
> +		return;
> +	}
> +

This is redundant wrt. the stricter compatibility check below, right? Or
did you find a call to nft_rule_to_iptables_command_state() which is not
guarded by nft_is_table_compatible()?

Anyway, I would add two checks to that function like so:

| if (!data) {
| 	ctx->errmsg = "missing bitwise xor attribute";
| 	return;
| }

(And the same for bitwise mask.) It will sanitize the function's code
irrespective of expression content, readers won't have to be aware of
(and rely upon) bitwise expression semantics with NFTNL_EXPR_BITWISE_OP
attribute value being zero.

>  	if (!sreg)
>  		return;
>  
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 85080a6d..661fac29 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -4029,7 +4029,6 @@ static const char *supported_exprs[] = {
>  	"payload",
>  	"meta",
>  	"cmp",
> -	"bitwise",
>  	"counter",
>  	"immediate",
>  	"lookup",
> @@ -4056,6 +4055,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) == 0 /* empty or MASK_XOR */)

'== NFT_BITWISE_MASK_XOR' and drop the comment.

Thanks, Phil