netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nft] json: make sure timeout list is initialised
@ 2025-03-21 11:53 Florian Westphal
  2025-03-21 12:45 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 2+ messages in thread
From: Florian Westphal @ 2025-03-21 11:53 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

On parser error, obj_free will iterate this list.
Included json bogon crashes due to null deref because
list head initialisation did not yet happen.

Fixes: c82a26ebf7e9 ("json: Add ct timeout support")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_json.c                             |  2 +-
 tests/shell/testcases/bogons/assert_failures  | 35 +++++++++---
 .../bogons/nft-j-f/ct_timeout_null_crash      | 54 +++++++++++++++++++
 3 files changed, 84 insertions(+), 7 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-j-f/ct_timeout_null_crash

diff --git a/src/parser_json.c b/src/parser_json.c
index 17bc38b565ae..dd085d78d0dd 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3722,6 +3722,7 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		break;
 	case NFT_OBJECT_CT_TIMEOUT:
 		cmd_obj = CMD_OBJ_CT_TIMEOUT;
+		init_list_head(&obj->ct_timeout.timeout_list);
 		obj->type = NFT_OBJECT_CT_TIMEOUT;
 		if (!json_unpack(root, "{s:s}", "protocol", &tmp)) {
 			if (!strcmp(tmp, "tcp")) {
@@ -3740,7 +3741,6 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		}
 		obj->ct_timeout.l3proto = l3proto;
 
-		init_list_head(&obj->ct_timeout.timeout_list);
 		if (json_parse_ct_timeout_policy(ctx, root, obj))
 			goto err_free_obj;
 		break;
diff --git a/tests/shell/testcases/bogons/assert_failures b/tests/shell/testcases/bogons/assert_failures
index 3dee63b3f97b..74e162ad476c 100755
--- a/tests/shell/testcases/bogons/assert_failures
+++ b/tests/shell/testcases/bogons/assert_failures
@@ -1,6 +1,8 @@
 #!/bin/bash
 
 dir=$(dirname $0)/nft-f/
+jsondir=$(dirname $0)/nft-j-f/
+
 tmpfile=$(mktemp)
 
 cleanup()
@@ -10,18 +12,39 @@ cleanup()
 
 trap cleanup EXIT
 
-for f in $dir/*; do
-	echo "Check $f"
-	$NFT --check -f "$f" 2> "$tmpfile"
+die_on_error()
+{
+	local rv="$1"
+	local fname="$2"
 
-	if [ $? -ne 1 ]; then
-		echo "Bogus input file $f did not cause expected error code" 1>&2
+	if [ $rv -ne 1 ]; then
+		echo "Bogus input file $fname did not cause expected error code" 1>&2
 		exit 111
 	fi
 
 	if grep AddressSanitizer "$tmpfile"; then
-		echo "Address sanitizer splat for $f" 1>&2
+		echo "Address sanitizer splat for $fname" 1>&2
 		cat "$tmpfile"
 		exit 111
 	fi
+}
+
+for f in $dir/*; do
+	echo "Check $f"
+	$NFT --check -f "$f" 2> "$tmpfile"
+
+	die_on_error $? "$f"
+done
+
+if [ "$NFT_TEST_HAVE_json" = "n" ];then
+	# Intentionally do not skip if we lack json input,
+	# we ran all the tests that we could.
+	exit 0
+fi
+
+for f in $jsondir/*; do
+	echo "Check json input $f"
+	$NFT --check -j -f "$f" 2> "$tmpfile"
+
+	die_on_error $?
 done
diff --git a/tests/shell/testcases/bogons/nft-j-f/ct_timeout_null_crash b/tests/shell/testcases/bogons/nft-j-f/ct_timeout_null_crash
new file mode 100644
index 000000000000..c8c662e93b8c
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-j-f/ct_timeout_null_crash
@@ -0,0 +1,54 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "ip",
+        "name": "filter",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "filter",
+        "name": "c",
+        "handle": 0
+      }
+    },
+    {
+      "ct timeout": {
+        "family": "ip",
+        "name": "cttime",
+        "table": "filter",
+        "handle": 0,
+        "protocol": "Xcp",
+        "l3proto": "ip",
+        "policy": {
+          "established": 123,
+          "close": 12
+        }
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "filter",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "ct timeout": "cttime"
+          }
+        ]
+      }
+    }
+  ]
+}
+
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH nft] json: make sure timeout list is initialised
  2025-03-21 11:53 [PATCH nft] json: make sure timeout list is initialised Florian Westphal
@ 2025-03-21 12:45 ` Pablo Neira Ayuso
  0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2025-03-21 12:45 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Fri, Mar 21, 2025 at 12:53:40PM +0100, Florian Westphal wrote:
> On parser error, obj_free will iterate this list.
> Included json bogon crashes due to null deref because
> list head initialisation did not yet happen.
> 
> Fixes: c82a26ebf7e9 ("json: Add ct timeout support")
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-03-21 12:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 11:53 [PATCH nft] json: make sure timeout list is initialised Florian Westphal
2025-03-21 12:45 ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).