@@ -447,6 +447,16 @@ static int expr_evaluate_integer(struct eval_ctx *ctx, struct expr **exprp)
return -1;
}
+ /* mpz_export() ignores the sign, so "-1" would silently become 1. */
+ if (mpz_sgn(expr->value) < 0) {
+ valstr = mpz_get_str(NULL, 10, expr->value);
+ expr_error(ctx->msgs, expr,
+ "Value %s is negative, expecting an unsigned value",
+ valstr);
+ nft_gmp_free(valstr);
+ return -1;
+ }
+
if (ctx->stmt_len > ctx->ectx.len)
masklen = ctx->stmt_len;
else
@@ -56,6 +56,8 @@ meta mark and 0x03 == 0x01;ok;meta mark & 0x00000003 == 0x00000001
meta mark and 0x03 != 0x01;ok;meta mark & 0x00000003 != 0x00000001
meta mark 0x10;ok;meta mark 0x00000010
meta mark != 0x10;ok;meta mark != 0x00000010
+meta mark "-1";fail
+meta mark "-4294967295";fail
meta mark 0xffffff00/24;ok;meta mark & 0xffffff00 == 0xffffff00
meta mark or 0x03 == 0x01;ok;meta mark | 0x00000003 == 0x00000001
new file mode 100644
new file mode 100755
@@ -0,0 +1,70 @@
+#!/bin/bash
+
+# mpz_export() drops the sign, so a negative value used to land as its
+# absolute value: "-1" became 1, from every frontend spelling. Chain and
+# flowtable priorities are signed and must keep working.
+
+set -e
+
+$NFT add table ip t
+$NFT add set ip t s '{ type mark; }'
+
+if $NFT add element ip t s '{ "-1" }' 2>/dev/null; then
+ echo "E: accepted a negative set element" >&2
+ $NFT list set ip t s >&2
+ exit 1
+fi
+
+# a rejected add must not have committed anything
+out=$($NFT list set ip t s)
+case "$out" in
+*elements*)
+ echo "E: something was stored by the failed add" >&2
+ echo "$out" >&2
+ exit 1
+ ;;
+esac
+
+$NFT add chain ip t c
+
+if $NFT add rule ip t c meta mark '"-1"' 2>/dev/null; then
+ echo "E: accepted a negative value in a rule" >&2
+ exit 1
+fi
+
+# the json frontend must reject a negative element as a string and as a
+# bare number, both used to land as element 1
+if [ "$NFT_TEST_HAVE_json" != n ]; then
+ if echo '{"nftables":[{"add":{"element":{"family":"ip","table":"t","name":"s","elem":["-1"]}}}]}' | $NFT -j -f - 2>/dev/null; then
+ echo "E: json accepted a negative element as a string" >&2
+ exit 1
+ fi
+ if echo '{"nftables":[{"add":{"element":{"family":"ip","table":"t","name":"s","elem":[-1]}}}]}' | $NFT -j -f - 2>/dev/null; then
+ echo "E: json accepted a negative element as a number" >&2
+ exit 1
+ fi
+ out=$($NFT list set ip t s)
+ case "$out" in
+ *elements*)
+ echo "E: something was stored by the failed json adds" >&2
+ echo "$out" >&2
+ exit 1
+ ;;
+ esac
+fi
+
+# priorities are signed: every spelling of a negative one must keep working
+$NFT add chain ip t c1 '{ type filter hook prerouting priority -300; }'
+$NFT add chain ip t c2 '{ type filter hook prerouting priority filter - 10; }'
+
+$NFT -f - <<'NFT'
+define p = -300
+table ip t2 {
+ chain c { type filter hook prerouting priority $p; policy accept; }
+}
+NFT
+
+# flowtables share evaluate_priority(); --check keeps the kernel out of it
+$NFT -c add flowtable ip t f '{ hook ingress priority -300; }'
+
+exit 0
expr_evaluate_integer() only tests the upper bound, so a negative value passes the range check and mpz_export() then drops the sign: # nft add element ip t m { "-1" } # nft list set ip t m table ip t { set m { type mark elements = { 0x00000001 } } } The json frontend has it twice over: "elem": ["-1"] and the bare number "elem": [-1] are both accepted and land as element 1 the same way. The error string has read "Value %s exceeds valid range 0-%s" since the check was added, so the contract was already unsigned; only the upper half of it was enforced. The result is not a wrap either: "-1" gives 1 while "-4294967295" gives 0xffffffff. Reject any negative value. Chain and flowtable priorities are signed but never reach this check as a negative mpz: bare and json numeric priorities are built as raw C ints by both frontends, symbols are rebuilt from atoi() in priority_type_parse(), and the name-plus-offset forms are computed as C ints in evaluate_priority(). Fixes: cb7cb885d65e ("evaluate: add expr_evaluate_integer()") Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> --- v2: drop the priority_type special case, it can never see a negative mpz (Phil, Pablo); name the json string and number forms in the message and cover them plus flowtable priority in the test src/evaluate.c | 10 +++ tests/py/any/meta.t | 2 + .../parsing/dumps/negative_values_0.nodump | 0 .../shell/testcases/parsing/negative_values_0 | 70 +++++++++++++++++++ 4 files changed, 82 insertions(+) create mode 100644 tests/shell/testcases/parsing/dumps/negative_values_0.nodump create mode 100755 tests/shell/testcases/parsing/negative_values_0 base-commit: 4f54425ea59250dcd02518ef4a50e6c0b7c162bf