From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 26 May 2022 19:19:22 +0000 (-0400)
Subject: disallow lists and structural types in comparisons
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41c187c1745faaa443683bfdc76e815d0f9423e7;p=thirdparty%2Ffreeradius-server.git

disallow lists and structural types in comparisons
---

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index bb6f2690954..ee19cfc128e 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -1219,6 +1219,32 @@ static fr_table_num_ordered_t const expr_assignment_op_table[] = {
 };
 static size_t const expr_assignment_op_table_len = NUM_ELEMENTS(expr_assignment_op_table);
 
+static bool valid_type(xlat_exp_t *node)
+{
+	fr_dict_attr_t const *da;
+
+	if (node->type != XLAT_TMPL) return true;
+
+	if (tmpl_is_list(node->vpt)) {
+	list:
+		fr_strerror_const("Cannot use list references in condition");
+		return false;
+	}
+
+	if (!tmpl_is_attr(node->vpt)) return true;
+
+	da = tmpl_da(node->vpt);
+	if (fr_type_is_structural(da->type)) {
+		if (da->dict == fr_dict_internal()) goto list;
+
+		fr_strerror_const("Cannot use structural types in condition");
+		fprintf(stderr, "FAIL %d\n", __LINE__);
+		return false;
+	}
+
+	return true;
+}
+
 /** Tokenize a mathematical operation.
  *
  *	(EXPR)
@@ -1236,7 +1262,7 @@ static ssize_t tokenize_expression(xlat_exp_head_t *head, xlat_exp_t **out, fr_s
 	xlat_t		*func = NULL;
 	fr_token_t	op;
 	ssize_t		slen;
-	fr_sbuff_marker_t  marker;
+	fr_sbuff_marker_t  m_lhs, m_op, m_rhs;
 	fr_sbuff_t	our_in = FR_SBUFF(in);
 	xlat_exp_head_t *args;
 
@@ -1244,6 +1270,8 @@ static ssize_t tokenize_expression(xlat_exp_head_t *head, xlat_exp_t **out, fr_s
 
 	fr_sbuff_skip_whitespace(&our_in);
 
+	fr_sbuff_marker(&m_lhs, &our_in);
+
 	/*
 	 *	Get the LHS of the operation.
 	 */
@@ -1294,7 +1322,7 @@ redo:
 	/*
 	 *	Remember where we were after parsing the LHS.
 	 */
-	fr_sbuff_marker(&marker, &our_in);
+	fr_sbuff_marker(&m_op, &our_in);
 
 	/*
 	 *	Get the operator.
@@ -1335,7 +1363,7 @@ redo:
 		fr_assert(0);
 
 #else
-		fr_sbuff_set(&our_in, &marker);
+		fr_sbuff_set(&our_in, &m_op);
 		fr_strerror_printf("Invalid operator '%s' - regular expressions are not supported in this build.", fr_tokens[op]);
 		FR_SBUFF_ERROR_RETURN_ADJ(&our_in, -slen);
 #endif
@@ -1343,7 +1371,7 @@ redo:
 
 	if (!binary_ops[op].str) {
 		fr_strerror_printf("Invalid operator");
-		fr_sbuff_set(&our_in, &marker);
+		fr_sbuff_set(&our_in, &m_op);
 		return -fr_sbuff_used(&our_in);
 	}
 
@@ -1356,10 +1384,13 @@ redo:
 	 *	take care of continuing.
 	 */
 	if (precedence[op] <= precedence[prev]) {
-		fr_sbuff_set(&our_in, &marker);
+		fr_sbuff_set(&our_in, &m_op);
 		goto done;
 	}
 
+	fr_sbuff_skip_whitespace(&our_in);
+	fr_sbuff_marker(&m_rhs, &our_in);
+
 	/*
 	 *	We now parse the RHS, allowing a (perhaps different) cast on the RHS.
 	 */
@@ -1397,6 +1428,21 @@ redo:
 	 *	optimization minimizes the amount of run-time work we have to do.
 	 */
 
+	/*
+	 *	Remove invalid comparisons.
+	 */
+	if (fr_equality_op[op]) {
+		if (!valid_type(lhs)) {
+			fr_sbuff_set(&our_in, &m_lhs);
+			return -fr_sbuff_used(&our_in);
+		}
+
+		if (!valid_type(rhs)) {
+			fr_sbuff_set(&our_in, &m_rhs);
+			return -fr_sbuff_used(&our_in);
+		}
+	}
+
 	/*
 	 *	Create the function node, with the LHS / RHS arguments.
 	 */
diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt
index dc0c2f39653..55fbcd6c214 100644
--- a/src/tests/unit/xlat/cond_base.txt
+++ b/src/tests/unit/xlat/cond_base.txt
@@ -122,14 +122,14 @@ match ('handled' && (&Packet-Type == Access-Challenge))
 #
 #  @todo - fix list comparisons
 #
-#xlat_purify &reply == &request
-#match ERROR offset 0: Cannot use list references in condition
+xlat_purify &reply == &request
+match ERROR offset 0: Cannot use list references in condition
 
-#xlat_purify &reply == "hello"
-#match ERROR offset 0: Cannot use list references in condition
+xlat_purify &reply == "hello"
+match ERROR offset 0: Cannot use list references in condition
 
-#xlat_purify "hello" == &reply
-#match ERROR offset 11: Cannot use list references in condition
+xlat_purify "hello" == &reply
+match ERROR offset 11: Cannot use list references in condition
 
 
 #
@@ -174,7 +174,7 @@ match ((ipaddr)&Filter-Id == &Framed-IP-Address)
 #  @todo - perhaps better errors for casts?
 #
 xlat_purify <ipaddr>&Filter-Id == <blerg>&Framed-IP-Address
-match ERROR offset 22: No operand found.  Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value
+match ERROR offset 23: No operand found.  Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value
 
 xlat_purify <blerg>&Filter-Id == "foo"
 match ERROR offset 1: No operand found.  Expected &ref, literal, 'quoted literal', "%{expansion}", or enum value
@@ -565,9 +565,8 @@ match ERROR offset 17: Unknown attributes not allowed here
 xlat_purify &User-Name[a] == 'bob'
 match ERROR offset 11: Invalid array index
 
-# @todo - really 25?
 xlat_purify &User-Name == &Filter-Id[a]
-match ERROR offset 24: Invalid array index
+match ERROR offset 25: Invalid array index
 
 #
 #  Bounds checks...
@@ -695,4 +694,4 @@ xlat_purify (&User-Name == "bob") && ((&User-Password == "bob") || &EAP-Message)
 match ((&User-Name == "bob") && ((&User-Password == "bob") || &EAP-Message))
 
 count
-match 261
+match 267