Blob Blame History Raw
From c40b8c22beef6011d79c60742bc4043d681198c1 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Fri, 12 Oct 2018 17:23:24 +0200
Subject: [PATCH] parser_json: Fix for ineffective family value checks

Since handle->family is unsigned, checking for value < 0 never yields
true. Overcome this by changing parse_family() to return an error code
and write the parsed family value into a pointer passed as parameter.

The above change required a bit more cleanup to avoid passing pointers
to signed variables to the function. Also leverage json_parse_family() a
bit more to reduce code side.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit c7a5401943df8b6b96f6b5eedd9a1e0013e01d86)

Conflicts:
	src/parser_json.c
-> Missing ct timeout support
-> missing tproxy support in JSON
-> Missing ipsec expression

Signed-off-by: Phil Sutter <psutter@redhat.com>
---
 src/parser_json.c | 130 ++++++++++++++++++++++------------------------
 1 file changed, 62 insertions(+), 68 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index af57b3025a104..30de17f8a1e26 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -173,7 +173,7 @@ static int json_unpack_stmt(struct json_ctx *ctx, json_t *root,
 	return 1;
 }
 
-static int parse_family(const char *name)
+static int parse_family(const char *name, uint32_t *family)
 {
 	unsigned int i;
 	struct {
@@ -188,13 +188,37 @@ static int parse_family(const char *name)
 		{ "netdev", NFPROTO_NETDEV }
 	};
 
+	assert(family);
+
 	for (i = 0; i < array_size(family_tbl); i++) {
-		if (!strcmp(name, family_tbl[i].name))
-			return family_tbl[i].val;
+		if (strcmp(name, family_tbl[i].name))
+			continue;
+
+		*family = family_tbl[i].val;
+		return 0;
 	}
 	return -1;
 }
 
+static int json_parse_family(struct json_ctx *ctx, json_t *root)
+{
+	const char *family;
+
+	if (!json_unpack(root, "{s:s}", "family", &family)) {
+		uint32_t familyval;
+
+		if (parse_family(family, &familyval) ||
+		    (familyval != NFPROTO_IPV6 &&
+		     familyval != NFPROTO_IPV4)) {
+			json_error(ctx, "Invalid family '%s'.", family);
+			return -1;
+		}
+		return familyval;
+	}
+
+	return NFPROTO_UNSPEC;
+}
+
 static bool is_keyword(const char *keyword)
 {
 	const char *keywords[] = {
@@ -594,19 +618,15 @@ static struct expr *json_parse_rt_expr(struct json_ctx *ctx,
 		{ "nexthop", NFT_RT_NEXTHOP4 },
 		{ "mtu", NFT_RT_TCPMSS },
 	};
-	unsigned int i, familyval = NFPROTO_UNSPEC;
-	const char *key, *family = NULL;
+	const char *key;
+	unsigned int i;
+	int familyval;
 
 	if (json_unpack_err(ctx, root, "{s:s}", "key", &key))
 		return NULL;
-	if (!json_unpack(root, "{s:s}", "family", &family)) {
-		familyval = parse_family(family);
-		if (familyval != NFPROTO_IPV4 &&
-		    familyval != NFPROTO_IPV6) {
-			json_error(ctx, "Invalid RT family '%s'.", family);
-			return NULL;
-		}
-	}
+	familyval = json_parse_family(ctx, root);
+	if (familyval < 0)
+		return NULL;
 
 	for (i = 0; i < array_size(rt_key_tbl); i++) {
 		int val = rt_key_tbl[i].val;
@@ -653,9 +673,9 @@ static bool ct_key_is_dir(enum nft_ct_keys key)
 static struct expr *json_parse_ct_expr(struct json_ctx *ctx,
 				       const char *type, json_t *root)
 {
-	const char *key, *dir, *family;
+	const char *key, *dir;
 	unsigned int i;
-	int dirval = -1, familyval = NFPROTO_UNSPEC, keyval = -1;
+	int dirval = -1, familyval, keyval = -1;
 
 	if (json_unpack_err(ctx, root, "{s:s}", "key", &key))
 		return NULL;
@@ -672,14 +692,9 @@ static struct expr *json_parse_ct_expr(struct json_ctx *ctx,
 		return NULL;
 	}
 
-	if (!json_unpack(root, "{s:s}", "family", &family)) {
-		familyval = parse_family(family);
-		if (familyval != NFPROTO_IPV4 &&
-		    familyval != NFPROTO_IPV6) {
-			json_error(ctx, "Invalid CT family '%s'.", family);
-			return NULL;
-		}
-	}
+	familyval = json_parse_family(ctx, root);
+	if (familyval < 0)
+		return NULL;
 
 	if (!json_unpack(root, "{s:s}", "dir", &dir)) {
 		if (!strcmp(dir, "original")) {
@@ -1562,7 +1577,6 @@ static struct stmt *json_parse_fwd_stmt(struct json_ctx *ctx,
 					const char *key, json_t *value)
 {
 	json_t *jaddr, *jdev;
-	const char *family;
 	struct stmt *stmt;
 	int familyval;
 
@@ -1577,21 +1591,15 @@ static struct stmt *json_parse_fwd_stmt(struct json_ctx *ctx,
 		goto out_err;
 	}
 
-	if (json_unpack(value, "{s:s, s:o}",
-			"family", &family, "addr", &jaddr))
-		return stmt;
-
-	familyval = parse_family(family);
-	switch (familyval) {
-	case NFPROTO_IPV4:
-	case NFPROTO_IPV6:
-		stmt->fwd.family = familyval;
-		break;
-	default:
-		json_error(ctx, "Invalid fwd family value '%s'.", family);
+	familyval = json_parse_family(ctx, value);
+	if (familyval < 0)
 		goto out_err;
-	}
 
+	if (familyval == NFPROTO_UNSPEC ||
+	    json_unpack(value, "{s:o}", "addr", &jaddr))
+		return stmt;
+
+	stmt->fwd.family = familyval;
 	stmt->fwd.addr = json_parse_stmt_expr(ctx, jaddr);
 	if (!stmt->fwd.addr) {
 		json_error(ctx, "Invalid fwd addr value.");
@@ -2137,8 +2145,7 @@ static struct cmd *json_parse_cmd_add_table(struct json_ctx *ctx, json_t *root,
 		json_error(ctx, "Either name or handle required to delete a table.");
 		return NULL;
 	}
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2178,8 +2185,7 @@ static struct cmd *json_parse_cmd_add_chain(struct json_ctx *ctx, json_t *root,
 		json_error(ctx, "Either name or handle required to delete a chain.");
 		return NULL;
 	}
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2240,8 +2246,7 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		 json_unpack_err(ctx, root, "{s:I}", "handle", &h.handle.id))
 		return NULL;
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2347,8 +2352,7 @@ static struct cmd *json_parse_cmd_add_set(struct json_ctx *ctx, json_t *root,
 		return NULL;
 	}
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2459,8 +2463,7 @@ static struct cmd *json_parse_cmd_add_element(struct json_ctx *ctx,
 			    "elem", &tmp))
 		return NULL;
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2523,8 +2526,7 @@ static struct cmd *json_parse_cmd_add_flowtable(struct json_ctx *ctx,
 			    "name", &h.flowtable))
 		return NULL;
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2568,6 +2570,7 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 					     enum cmd_obj cmd_obj)
 {
 	const char *family, *tmp, *rate_unit = "packets", *burst_unit = "bytes";
+	uint32_t l3proto = NFPROTO_IPV4;
 	struct handle h = { 0 };
 	struct obj *obj;
 	int inv = 0;
@@ -2588,8 +2591,7 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		return NULL;
 	}
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2647,18 +2649,13 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 				return NULL;
 			}
 		}
-		if (!json_unpack(root, "{s:s}", "l3proto", &tmp)) {
-			int family = parse_family(tmp);
-
-			if (family < 0) {
-				json_error(ctx, "Invalid ct helper l3proto '%s'.", tmp);
-				obj_free(obj);
-				return NULL;
-			}
-			obj->ct_helper.l3proto = family;
-		} else {
-			obj->ct_helper.l3proto = NFPROTO_IPV4;
+		if (!json_unpack(root, "{s:s}", "l3proto", &tmp) &&
+		    parse_family(tmp, &l3proto)) {
+			json_error(ctx, "Invalid ct helper l3proto '%s'.", tmp);
+			obj_free(obj);
+			return NULL;
 		}
+		obj->ct_helper.l3proto = l3proto;
 		break;
 	case CMD_OBJ_LIMIT:
 		obj->type = NFT_OBJECT_LIMIT;
@@ -2770,8 +2767,7 @@ static struct cmd *json_parse_cmd_replace(struct json_ctx *ctx,
 		h.handle.id = 0;
 	}
 
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
@@ -2825,8 +2821,7 @@ static struct cmd *json_parse_cmd_list_multiple(struct json_ctx *ctx,
 	const char *tmp;
 
 	if (!json_unpack(root, "{s:s}", "family", &tmp)) {
-		h.family = parse_family(tmp);
-		if (h.family < 0) {
+		if (parse_family(tmp, &h.family)) {
 			json_error(ctx, "Unknown family '%s'.", tmp);
 			return NULL;
 		}
@@ -2981,8 +2976,7 @@ static struct cmd *json_parse_cmd_rename(struct json_ctx *ctx,
 			    "name", &h.chain.name,
 			    "newname", &newname))
 		return NULL;
-	h.family = parse_family(family);
-	if (h.family < 0) {
+	if (parse_family(family, &h.family)) {
 		json_error(ctx, "Unknown family '%s'.", family);
 		return NULL;
 	}
-- 
2.19.0