From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 12 Jan 2021 13:12:38 +0000 (-0500)
Subject: hoist more sanity checks to cond_promote_values()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=868c856a31ce6e6271f54cbf2a396fc8841a5a03;p=thirdparty%2Ffreeradius-server.git

hoist more sanity checks to cond_promote_values()

and ensure that for regexes, LHS is always cast to string.

This ensures less work for the run-time evaluator.
---

diff --git a/src/lib/server/cond_tokenize.c b/src/lib/server/cond_tokenize.c
index fccd2a2dd73..edb11e2435e 100644
--- a/src/lib/server/cond_tokenize.c
+++ b/src/lib/server/cond_tokenize.c
@@ -244,30 +244,52 @@ int fr_cond_promote_types(fr_cond_t *c, fr_sbuff_t *in, fr_sbuff_marker_t *m_lhs
 	/*
 	 *	Regular expressions have their own casting rules.
 	 */
-	if (tmpl_is_regex(c->data.map->rhs)) {
+	if (tmpl_contains_regex(c->data.map->rhs)) {
 		fr_assert((c->data.map->op == T_OP_REG_EQ) || (c->data.map->op == T_OP_REG_NE));
 		fr_assert(!tmpl_is_list(c->data.map->lhs));
+		fr_assert(c->data.map->rhs->cast == FR_TYPE_INVALID);
 
 		/*
-		 *	If the LHS is unresolved data, then cast it to
-		 *	a string.
+		 *	Can't use casts with regular expressions, on
+		 *	LHS or RHS.  Instead, the regular expression
+		 *	returns a true/false match.
+		 *
+		 *	It's OK to have a redundant cast to string on
+		 *	the LHS.  We also allow a cast to octets, in
+		 *	which case we do a binary regex comparison.
+		 *
+		 *	@todo - ensure that the regex interpreter is
+		 *	binary-safe!
 		 */
-		if (tmpl_is_unresolved(c->data.map->lhs)) {
-			if (tmpl_cast_in_place(c->data.map->lhs, FR_TYPE_STRING, NULL) < 0) return -1;
-
-			(void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_INVALID);
-			return 0;
+		cast_type = c->data.map->lhs->cast;
+		if (cast_type != FR_TYPE_INVALID) {
+			if ((cast_type != FR_TYPE_STRING) &&
+			    (cast_type != FR_TYPE_OCTETS)) {
+				fr_strerror_const("Invalid cast used with regular expression");
+				if (in) fr_sbuff_set(in, m_lhs);
+				return -1;
+			}
+		} else {
+			cast_type = FR_TYPE_STRING;
 		}
 
 		/*
-		 *	The LHS doesn't have a cast, so we coerce it
-		 *	to string.  This means one fewer check at
-		 *	run-time.
+		 *	Ensure that the data is cast appropriately.
 		 */
-		if (c->data.map->lhs->cast == FR_TYPE_INVALID) {
-			(void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_STRING);
+		if (tmpl_is_unresolved(c->data.map->lhs) || tmpl_is_data(c->data.map->lhs)) {
+			if (tmpl_cast_in_place(c->data.map->lhs, FR_TYPE_STRING, NULL) < 0) {
+				if (in) fr_sbuff_set(in, m_lhs);
+				return -1;
+			}
+
+			(void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_INVALID);
 		}
 
+		/*
+		 *	Force a cast to 'string', so the conditional
+		 *	evaluator has less work to do.
+		 */
+		(void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_STRING);
 		return 0;
 	}
 #endif
@@ -1041,6 +1063,12 @@ static ssize_t cond_tokenize_operand(fr_cond_t *c, tmpl_t **out,
 	 *	at the start of the flags.
 	 */
 	if (type == T_SOLIDUS_QUOTED_STRING) {
+		if (cast != FR_TYPE_INVALID) {
+			fr_strerror_const("Invalid cast used with regular expression");
+			fr_sbuff_set(&our_in, &m);
+			goto error;
+		}
+
 		if (!tmpl_contains_regex(vpt)) {
 			fr_strerror_const("Expected regex");
 			fr_sbuff_set(&our_in, &m);
@@ -1391,34 +1419,6 @@ static ssize_t cond_tokenize(TALLOC_CTX *ctx, fr_cond_t **out,
 				fr_sbuff_set(&our_in, &m_rhs);
 				goto error;
 			}
-
-			/*
-			 *	Can't use casts with regular
-			 *	expressions, on LHS or RHS.  Instead,
-			 *	the regular expression returns a
-			 *	true/false match.
-			 *
-			 *	It's OK to have a redundant cast to
-			 *	string on the LHS.  We also allow a
-			 *	cast to octets, in which case we do a
-			 *	binary regex comparison.
-			 *
-			 *	@todo - ensure that the regex
-			 *	interpreter is binary-safe!
-			 */
-			if ((lhs->cast != FR_TYPE_INVALID) &&
-			    (lhs->cast != FR_TYPE_STRING) &&
-			    (lhs->cast != FR_TYPE_OCTETS)) {
-				fr_strerror_const("Invalid cast used with regular expression");
-				fr_sbuff_set(&our_in, &m_lhs);
-				goto error;
-			}
-
-			if (rhs->cast != FR_TYPE_INVALID) {
-				fr_strerror_const("Invalid cast used with regular expression");
-				fr_sbuff_set(&our_in, &m_rhs);
-				goto error;
-			}
 		}
 #endif
 
diff --git a/src/tests/unit/condition/regex.txt b/src/tests/unit/condition/regex.txt
index f61ef0b38b5..5062fc9c539 100644
--- a/src/tests/unit/condition/regex.txt
+++ b/src/tests/unit/condition/regex.txt
@@ -59,6 +59,22 @@ match &User-Name =~ /@|\\/
 condition &User-Name =~ /^([^\\]*)\\(.*)$/
 match &User-Name =~ /^([^\\]*)\\(.*)$/
 
+#
+#  Non-integer types get cast to string.
+#
+condition &Tmp-Integer-0 =~ /%{Tmp-Integer-0}/
+match <string>&Tmp-Integer-0 =~ /%{Tmp-Integer-0}/
+
+#
+#  Cannot add a bad cast
+#
+condition <integer>&Tmp-String-0 =~ /foo/
+match ERROR offset 9: Invalid cast used with regular expression
+
+condition &Tmp-String-0 =~ <integer>/foo/
+match ERROR offset 26: Invalid cast used with regular expression
+
+
 xlat %{1}
 match %{1}
 
@@ -69,4 +85,4 @@ condition &User-Name == /foo/
 match ERROR offset 14: Unexpected regular expression
 
 count
-match 37
+match 43