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.21.0