From 25ccb4587de81e1b348e4e2f5404e70097ea8ec9 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Mon, 28 May 2018 18:51:01 +0200
Subject: JSON: Review set elem expressions

* There is no need to prefix element-specific properties with 'elem_',
  they can't conflict.
* In json_parse_set_stmt(), searching for above properties is pointless
  since that's already done by called function.
* Fix potential NULL-pointer deref in json_parse_set_elem_expr_stmt():
  json_parse_flagged_expr() may return NULL.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/json.c                          |  6 +++---
 src/parser_json.c                   | 15 ++++-----------
 tests/py/ip/flowtable.t.json        |  2 +-
 tests/py/ip/flowtable.t.json.output | 24 ++++++++++++++++++++++++
 tests/py/ip6/flowtable.t.json       |  4 ++--
 5 files changed, 34 insertions(+), 17 deletions(-)
 create mode 100644 tests/py/ip/flowtable.t.json.output

diff --git a/src/json.c b/src/json.c
index e31e3132..83366df8 100644
--- a/src/json.c
+++ b/src/json.c
@@ -508,13 +508,13 @@ json_t *set_elem_expr_json(const struct expr *expr, struct output_ctx *octx)
 		root = json_pack("{s:o}", "val", root);
 
 		if (expr->timeout)
-			json_object_set_new(root, "elem_timeout",
+			json_object_set_new(root, "timeout",
 					    json_integer(expr->timeout / 1000));
 		if (expr->expiration)
-			json_object_set_new(root, "elem_expires",
+			json_object_set_new(root, "expires",
 					    json_integer(expr->expiration / 1000));
 		if (expr->comment)
-			json_object_set_new(root, "elem_comment",
+			json_object_set_new(root, "comment",
 					    json_string(expr->comment));
 		return json_pack("{s:o}", "elem", root);
 	}
diff --git a/src/parser_json.c b/src/parser_json.c
index afcd10a3..fd60c59c 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1140,11 +1140,11 @@ static struct expr *json_parse_set_elem_expr(struct json_ctx *ctx,
 
 	expr = set_elem_expr_alloc(int_loc, expr);
 
-	if (!json_unpack(root, "{s:i}", "elem_timeout", &i))
+	if (!json_unpack(root, "{s:i}", "timeout", &i))
 		expr->timeout = i * 1000;
-	if (!json_unpack(root, "{s:i}", "elem_expires", &i))
+	if (!json_unpack(root, "{s:i}", "expires", &i))
 		expr->expiration = i * 1000;
-	if (!json_unpack(root, "{s:s}", "elem_comment", &expr->comment))
+	if (!json_unpack(root, "{s:s}", "comment", &expr->comment))
 		expr->comment = xstrdup(expr->comment);
 
 	return expr;
@@ -1298,7 +1298,7 @@ static struct expr *json_parse_set_elem_expr_stmt(struct json_ctx *ctx, json_t *
 {
 	struct expr *expr = json_parse_flagged_expr(ctx, CTX_F_SES, root);
 
-	if (expr->ops->type != EXPR_SET_ELEM)
+	if (expr && expr->ops->type != EXPR_SET_ELEM)
 		expr = set_elem_expr_alloc(int_loc, expr);
 
 	return expr;
@@ -1820,7 +1820,6 @@ static struct stmt *json_parse_set_stmt(struct json_ctx *ctx,
 	struct expr *expr, *expr2;
 	struct stmt *stmt;
 	json_t *elem;
-	uint64_t tmp;
 	int op;
 
 	if (json_unpack_err(ctx, value, "{s:s, s:o, s:s}",
@@ -1842,12 +1841,6 @@ static struct stmt *json_parse_set_stmt(struct json_ctx *ctx,
 		return NULL;
 	}
 
-	if (!json_unpack(elem, "{s:I}", "elem_timeout", &tmp))
-		expr->timeout = tmp * 1000;
-	if (!json_unpack(elem, "{s:I}", "elem_expires", &tmp))
-		expr->expiration = tmp * 1000;
-	json_unpack(elem, "{s:s}", "elem_comment", &expr->comment);
-
 	if (set[0] != '@') {
 		json_error(ctx, "Illegal set reference in set statement.");
 		expr_free(expr);
diff --git a/tests/py/ip/flowtable.t.json b/tests/py/ip/flowtable.t.json
index ca4b5f61..5e11172e 100644
--- a/tests/py/ip/flowtable.t.json
+++ b/tests/py/ip/flowtable.t.json
@@ -4,7 +4,7 @@
         "meter": {
             "key": {
                 "elem": {
-                    "elem_timeout": 30,
+                    "timeout": 30,
                     "val": {
                         "payload": {
                             "field": "saddr",
diff --git a/tests/py/ip/flowtable.t.json.output b/tests/py/ip/flowtable.t.json.output
new file mode 100644
index 00000000..004349a8
--- /dev/null
+++ b/tests/py/ip/flowtable.t.json.output
@@ -0,0 +1,24 @@
+# meter xyz { ip saddr timeout 30s counter}
+[
+    {
+        "meter": {
+            "key": {
+                "elem": {
+                    "timeout": 30,
+                    "val": {
+                        "payload": {
+                            "field": "saddr",
+                            "name": "ip"
+                        }
+                    }
+                }
+            },
+            "name": "xyz",
+	    "size": 65535,
+            "stmt": {
+                "counter": null
+            }
+        }
+    }
+]
+
diff --git a/tests/py/ip6/flowtable.t.json b/tests/py/ip6/flowtable.t.json
index e3d05339..863669eb 100644
--- a/tests/py/ip6/flowtable.t.json
+++ b/tests/py/ip6/flowtable.t.json
@@ -4,7 +4,7 @@
         "meter": {
             "key": {
                 "elem": {
-                    "elem_timeout": 600,
+                    "timeout": 600,
                     "val": {
                         "concat": [
                             {
@@ -34,7 +34,7 @@
         "meter": {
             "key": {
                 "elem": {
-                    "elem_timeout": 600,
+                    "timeout": 600,
                     "val": {
                         "concat": [
                             {
-- 
cgit v1.2.3