[PATCH nft] json: fix error protagation when parsing binop lhs/rhs

[PATCH nft] json: fix error protagation when parsing binop lhs/rhs

[Florian Westphal]

Malformed input returns NULL when decoding left/right side of binop.
This causes a NULL dereference in expr_evaluate_binop; left/right must
point to a valid expression.

Fix this in the parser, else would have to sprinkle NULL checks all over
the evaluation code.

After fix, loading the bogon yields:
internal:0:0-0: Error: Malformed object (too many properties): '{}'.
internal:0:0-0: Error: could not decode binop rhs, '<<'.
internal:0:0-0: Error: Invalid mangle statement value
internal:0:0-0: Error: Parsing expr array at index 1 failed.
internal:0:0-0: Error: Parsing command array at index 3 failed.

Fixes: 0ac39384fd9e ("json: Accept more than two operands in binary expressions")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_json.c                             | 14 ++++
 .../nft-j-f/binop_rhs_decode_error_crash      | 76 +++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash

diff --git a/src/parser_json.c b/src/parser_json.c
index 04d762741e4a..9d5ec2275b30 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1210,11 +1210,25 @@ static struct expr *json_parse_binop_expr(struct json_ctx *ctx,
 
 	if (json_array_size(root) > 2) {
 		left = json_parse_primary_expr(ctx, json_array_get(root, 0));
+		if (!left) {
+			json_error(ctx, "Failed to parse LHS of binop expression.");
+			return NULL;
+		}
 		right = json_parse_primary_expr(ctx, json_array_get(root, 1));
+		if (!right) {
+			json_error(ctx, "Failed to parse RHS of binop expression.");
+			expr_free(left);
+			return NULL;
+		}
 		left = binop_expr_alloc(int_loc, thisop, left, right);
 		for (i = 2; i < json_array_size(root); i++) {
 			jright = json_array_get(root, i);
 			right = json_parse_primary_expr(ctx, jright);
+			if (!right) {
+				json_error(ctx, "Failed to parse RHS of binop expression.");
+				expr_free(left);
+				return NULL;
+			}
 			left = binop_expr_alloc(int_loc, thisop, left, right);
 		}
 		return left;
diff --git a/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash b/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash
new file mode 100644
index 000000000000..c5de17111ff6
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash
@@ -0,0 +1,76 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "ip",
+        "name": "t",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "t",
+        "name": "c",
+        "handle": 0,
+        "type": "filter",
+        "hook": "output",
+        "prio": 0,
+        "policy": "accept"
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "t",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "meta": {
+                  "key": "oif"
+                }
+              },
+              "right": "lo"
+            }
+          },
+          {
+            "mangle": {
+              "key": {
+                "ct": {
+                  "key": "mark"
+                }
+              },
+              "value": {
+                "<<": [
+                  {
+                    "|": [
+                      {
+                        "meta": {
+                          "key": "mark"
+                        }
+                      },
+                      16
+                    ]
+                  },
+    {  },
+                  8
+                ]
+              }
+            }
+          }
+        ]
+      }
+    }
+  ]
+}
-- 
2.48.1

Re: [PATCH nft] json: fix error protagation when parsing binop lhs/rhs

[Pablo Neira Ayuso]

On Fri, Mar 28, 2025 at 03:49:04PM +0100, Florian Westphal wrote:
> Malformed input returns NULL when decoding left/right side of binop.
> This causes a NULL dereference in expr_evaluate_binop; left/right must
> point to a valid expression.
> 
> Fix this in the parser, else would have to sprinkle NULL checks all over
> the evaluation code.
> 
> After fix, loading the bogon yields:
> internal:0:0-0: Error: Malformed object (too many properties): '{}'.
> internal:0:0-0: Error: could not decode binop rhs, '<<'.
> internal:0:0-0: Error: Invalid mangle statement value
> internal:0:0-0: Error: Parsing expr array at index 1 failed.
> internal:0:0-0: Error: Parsing command array at index 3 failed.
> 
> Fixes: 0ac39384fd9e ("json: Accept more than two operands in binary expressions")
> Signed-off-by: Florian Westphal <fw@strlen.de>

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

one nitpick:

> diff --git a/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash b/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash
> new file mode 100644
> index 000000000000..c5de17111ff6
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-j-f/binop_rhs_decode_error_crash
[...]
> +              "value": {
> +                "<<": [
> +                  {
> +                    "|": [
> +                      {
> +                        "meta": {
> +                          "key": "mark"
> +                        }
> +                      },
> +                      16
> +                    ]
> +                  },
> +    {  },

Something strange here in this indent.

> +                  8
> +                ]
> +              }
> +            }
> +          }
> +        ]
> +      }
> +    }
> +  ]
> +}