From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 25 Dec 2023 15:01:37 +0000 (-0500)
Subject: more sanity checks on operators
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d9831f4a7cae9f20110c3dad7fc3fe8648d61fd;p=thirdparty%2Ffreeradius-server.git

more sanity checks on operators
---

diff --git a/src/lib/server/map.c b/src/lib/server/map.c
index acdd88b3f53..06c6cc198b4 100644
--- a/src/lib/server/map.c
+++ b/src/lib/server/map.c
@@ -527,15 +527,17 @@ ssize_t map_afrom_substr(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, fr_sbuf
 		fr_strerror_const("Invalid operator");
 		goto error_adj;
 	}
-
+	
 	/*
-	 *	This function is ONLY called for legacy operators.
+	 *	Validate operators for check items.
+	 *
+	 *	We can have comparison operators for reply items, as the rlm_attr_filter module
+	 *	uses that.
 	 *
-	 *	radius_legacy_map_cmp() and radius_legacy_map_apply() do not support complex
-	 *	comparisons or updates.
+	 *	However, we can't do comparisons on structural entries, except for existence checks.
 	 */
-	if (tmpl_attr_tail_da_is_structural(map->lhs)) {
-		if (fr_comparison_op[map->op]) {
+	if (!parent_p && tmpl_attr_tail_da_is_structural(map->lhs)) {
+		if (fr_comparison_op[map->op] && (map->op != T_OP_CMP_TRUE) && (map->op != T_OP_CMP_FALSE)) {
 			fr_sbuff_set(&our_in, &m_op);
 			fr_strerror_const("Comparison operators cannot be used inside of structural data types");
 			goto error;
@@ -563,6 +565,7 @@ ssize_t map_afrom_substr(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, fr_sbuf
 		case FR_TYPE_STRUCTURAL:
 			if ((map->op == T_OP_REG_EQ) || (map->op == T_OP_REG_NE)) {
 				fr_sbuff_set(&our_in, &m_op);
+				fr_assert(0);
 				fr_strerror_const("Regular expressions cannot be used for structural attributes");
 				goto error;
 			}
diff --git a/src/modules/rlm_attr_filter/rlm_attr_filter.c b/src/modules/rlm_attr_filter/rlm_attr_filter.c
index ce29cd97f8b..c8ff1fbae3a 100644
--- a/src/modules/rlm_attr_filter/rlm_attr_filter.c
+++ b/src/modules/rlm_attr_filter/rlm_attr_filter.c
@@ -137,6 +137,12 @@ static int attr_filter_getfile(TALLOC_CTX *ctx, module_inst_ctx_t const *mctx, c
 				return -1;
 			}
 
+			if (fr_assignment_op[map->op]) {
+				ERROR("%s[%d] Filter %s contains invalid operator '%s'",
+				      filename, entry->lineno, map->lhs->name, fr_tokens[map->op]);
+				return -1;
+			}
+
 			/*
 			 *	Make sure that bad things don't happen.
 			 */
diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c
index bbf6d3be99b..6a1c8e93f2d 100644
--- a/src/modules/rlm_files/rlm_files.c
+++ b/src/modules/rlm_files/rlm_files.c
@@ -203,7 +203,6 @@ static int getrecv_filename(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **
 		     map != NULL;
 		     map = next_map) {
 			next_map = map_list_next(&entry->reply, map);
-
 			if (!tmpl_is_attr(map->lhs)) {
 				ERROR("%s[%d] Left side of reply item %s is not an attribute",
 				      entry->filename, entry->lineno, map->lhs->name);
@@ -211,9 +210,9 @@ static int getrecv_filename(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **
 			}
 			da = tmpl_attr_tail_da(map->lhs);
 
-			if (map->op == T_OP_CMP_FALSE) {
-				ERROR("%s[%d] Invalid operator '!*' for reply item %s",
-				      entry->filename, entry->lineno, map->lhs->name);
+			if (fr_comparison_op[map->op] && (map->op != T_OP_LE) && (map->op != T_OP_GE)) {
+				ERROR("%s[%d] Invalid operator reply item %s %s ...",
+				      entry->filename, entry->lineno, map->lhs->name, fr_tokens[map->op]);
 				return -1;
 			}
 
diff --git a/src/tests/modules/attr_filter/filter b/src/tests/modules/attr_filter/filter
index 443b8f50f0c..8686fcba558 100644
--- a/src/tests/modules/attr_filter/filter
+++ b/src/tests/modules/attr_filter/filter
@@ -23,6 +23,7 @@ DEFAULT
 	Error-Cause =* ANY,
 	Reply-Message =* ANY,
 	Vendor-Specific.Microsoft.CHAP-Error =* ANY,
+	Digest-Attributes =* ANY,
 	Proxy-State =* ANY,
 	Error-Cause =* ANY,
 	User-Name =* ANY,