datatype: fix crash when using basetype instead of symbolic constants

PATCH New Pablo Neira Ayuso 1 participant 2 messages Nov 28, 2014 netfilter

lore

Share & tools

lore

Pablo Neira Ayuso

author maintainer

2014-11-28 20:08 UTC #0 lore raw

PATCH v2 1/2 datatype: fix crash when using basetype instead of symbolic constants

The following example:

 # nft add rule filter input ct state 8 accept
 Segmentation fault

leads to a crash because we have the following datatype relation:

ct_state -> bitmask -> integer

The bitmask, which is an intermediate basetype, has no parse() function, this leads to a crash in symbolic_constant_parse().

Patrick suggested to walk down the chain until we find a parser function.

Reported-by leroy christophe <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 src/datatype.c            |   33 ++++++++++++++++++++-------------
 tests/regression/any/ct.t |    1 +
 2 files changed, 21 insertions(+), 13 deletions(-)

src/datatype.c +20 ~~-13~~

index 5f976aa..7c9c3d4 100644
@@ -125,21 +125,28 @@ struct error_record *symbolic_constant_parse(const struct expr *sym,
 			break;
 	}
 
-	dtype = sym->dtype;
-	if (s->identifier == NULL) {
-		*res = NULL;
-		erec = sym->dtype->basetype->parse(sym, res);
-		if (erec != NULL)
-			return erec;
-		if (*res)
-			return NULL;
+	if (s->identifier != NULL)
+		goto out;
 
-		return error(&sym->location, "Could not parse %s", dtype->desc);
-	}
+	dtype = sym->dtype;
+	*res = NULL;
+	do {
+		if (dtype->basetype->parse) {
+			erec = dtype->basetype->parse(sym, res);
+			if (erec != NULL)
+				return erec;
+			if (*res)
+				return NULL;
+			goto out;
+		}
+	} while ((dtype = dtype->basetype));
 
-	*res = constant_expr_alloc(&sym->location, dtype,
-				   dtype->byteorder, dtype->size,
-				   constant_data_ptr(s->value, dtype->size));
+	return error(&sym->location, "Could not parse %s", sym->dtype->desc);
+out:
+	*res = constant_expr_alloc(&sym->location, sym->dtype,
+				   sym->dtype->byteorder, sym->dtype->size,
+				   constant_data_ptr(s->value,
+				   sym->dtype->size));
 	return NULL;
 }

tests/regression/any/ct.t +1 -0

index 7ce898d..79674ee 100644
@@ -13,6 +13,7 @@ ct state {new,established, related, untracked};ok
 - ct state != {new,established, related, untracked};ok
 ct state invalid drop;ok
 ct state established accept;ok
+ct state 8;ok;ct state new
 
 ct direction original;ok
 ct direction != original;ok

Signature · 1 line

1.7.10.4

Pablo Neira Ayuso

author maintainer

2014-11-28 20:08 UTC #1 lore raw

PATCH 2/2 datatype: relax datatype check in integer_type_parse()

Otherwise parsing with basetypes doesn't work. Now nft displays an error when the symbolic constant is not correct:

<cmdline>:1:29-31: Error: Could not parse conntrack state add rule test test ct state xxx accept

^^^

I also needed to revert the logic in ethertype_parse and pkttype_type_parse so we first look up for the symbol in the table, otherwise default to the basetype. Otherwise, the regression test were reporting a breakage in the ether type / pkttype.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 src/datatype.c              |    2 --
 src/meta.c                  |   16 ++--------------
 src/proto.c                 |    4 ++--
 tests/regression/any/ct.t   |    3 +++
 tests/regression/any/meta.t |    1 +
 5 files changed, 8 insertions(+), 18 deletions(-)

src/datatype.c +0 -2

index 7c9c3d4..1ace3fa 100644
@@ -280,8 +280,6 @@ static struct error_record *integer_type_parse(const struct expr *sym,
 	if (gmp_sscanf(sym->identifier, "%Zu%n", v, &len) != 1 ||
 	    (int)strlen(sym->identifier) != len) {
 		mpz_clear(v);
-		if (sym->dtype != &integer_type)
-			return NULL;
 		return error(&sym->location, "Could not parse %s",
 			     sym->dtype->desc);
 	}

src/meta.c +2 ~~-14~~

index 61dc5cf..de83391 100644
@@ -316,26 +316,14 @@ static struct error_record *pkttype_type_parse(const struct expr *sym,
 					       struct expr **res)
 {
 	struct error_record *erec;
-	const struct symbolic_constant *s;
-
-	for (s = pkttype_type_tbl.symbols; s->identifier != NULL; s++) {
-		if (!strcmp(sym->identifier, s->identifier)) {
-			*res = constant_expr_alloc(&sym->location, sym->dtype,
-						   sym->dtype->byteorder,
-						   sym->dtype->size,
-						   &s->value);
-			return NULL;
-		}
-	}
 
 	*res = NULL;
-	erec = sym->dtype->basetype->parse(sym, res);
+	erec = symbolic_constant_parse(sym, &pkttype_type_tbl, res);
 	if (erec != NULL)
 		return erec;
 	if (*res)
 		return NULL;
-
-	return symbolic_constant_parse(sym, &pkttype_type_tbl, res);
+	return sym->dtype->basetype->parse(sym, res);
 }
 
 static const struct datatype pkttype_type = {

src/proto.c +2 -2

index 6eb4bb2..94ecc58 100644
@@ -757,12 +757,12 @@ static struct error_record *ethertype_parse(const struct expr *sym,
 	struct error_record *erec;
 
 	*res = NULL;
-	erec = sym->dtype->basetype->parse(sym, res);
+	erec = symbolic_constant_parse(sym, &ethertype_tbl, res);
 	if (erec != NULL)
 		return erec;
 	if (*res)
 		return NULL;
-	return symbolic_constant_parse(sym, &ethertype_tbl, res);
+	return sym->dtype->basetype->parse(sym, res);
 }
 
 static void ethertype_print(const struct expr *expr)

tests/regression/any/ct.t +3 -0

index 79674ee..09f72ed 100644
@@ -14,6 +14,7 @@ ct state {new,established, related, untracked};ok
 ct state invalid drop;ok
 ct state established accept;ok
 ct state 8;ok;ct state new
+ct state xxx;fail
 
 ct direction original;ok
 ct direction != original;ok

@@ -21,12 +22,14 @@ ct direction reply;ok
 ct direction != reply;ok
 ct direction {reply, original};ok
 - ct direction != {reply, original};ok
+ct direction xxx;fail
 
 ct status expected;ok
 ct status != expected;ok
 ct status seen-reply;ok
 ct status != seen-reply;ok
 ct status {expected, seen-reply, assured, confirmed, dying};ok
+ct status xxx;fail
 
 # SYMBOL("snat", IPS_SRC_NAT)
 # SYMBOL("dnat", IPS_DST_NAT)

tests/regression/any/meta.t +1 -0

index b3f91c9..3acf953 100644
@@ -150,6 +150,7 @@ meta pkttype != unicast;ok;pkttype != unicast
 meta pkttype != multicast;ok;pkttype != multicast
 meta pkttype broadcastttt;fail
 -meta pkttype { broadcast, multicast} accept;ok
+meta pkttype xxx;fail
 
 meta cpu 1;ok;cpu 1
 meta cpu != 1;ok;cpu != 1

Signature · 1 line

1.7.10.4