From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 10 Mar 2025 12:31:03 +0000 (-0400)
Subject: add and use FR_VALUE_BOX_SAFE_FOR_ANY
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fda8378770af18eb19993d4a403b9120fef17fce;p=thirdparty%2Ffreeradius-server.git

add and use FR_VALUE_BOX_SAFE_FOR_ANY

which lets us *not* escape data which is taken from the
configuration files

disable the regex escape test for now.  We don't want a regex.safe()
function.  We want a "escape this string, even tho it's value is safe"
function
---

diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c
index 32adc935a19..2a8a7ca8803 100644
--- a/src/lib/server/cf_file.c
+++ b/src/lib/server/cf_file.c
@@ -1543,7 +1543,8 @@ static CONF_ITEM *process_if(cf_stack_t *stack)
 			.list_def = request_attr_request,
 			.allow_unresolved = true,
 			.allow_unknown = true
-		}
+		},
+		.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 	};
 
 	/*
diff --git a/src/lib/server/cf_parse.c b/src/lib/server/cf_parse.c
index 13d917df95f..1854df4ed55 100644
--- a/src/lib/server/cf_parse.c
+++ b/src/lib/server/cf_parse.c
@@ -220,7 +220,8 @@ int cf_pair_parse_value(TALLOC_CTX *ctx, void *out, UNUSED void *base, CONF_ITEM
 							.allow_unknown = true,
 							.allow_unresolved = true,
 							.allow_foreign = true,
-						}
+						},
+						.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 					};
 		fr_sbuff_t		sbuff = FR_SBUFF_IN(cp->value, strlen(cp->value));
 
diff --git a/src/lib/server/users_file.c b/src/lib/server/users_file.c
index a769b8579b7..9f6345918a6 100644
--- a/src/lib/server/users_file.c
+++ b/src/lib/server/users_file.c
@@ -273,7 +273,9 @@ static int pairlist_read_internal(TALLOC_CTX *ctx, fr_dict_t const *dict, char c
 			.prefix = TMPL_ATTR_REF_PREFIX_AUTO,
 			.list_def = request_attr_request,
 			.list_presence = TMPL_ATTR_LIST_ALLOW,
-		}
+		},
+		.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
+
 	};
 	rhs_rules = (tmpl_rules_t) {
 		.attr = {
@@ -282,7 +284,8 @@ static int pairlist_read_internal(TALLOC_CTX *ctx, fr_dict_t const *dict, char c
 			.list_def = request_attr_request,
 			.list_presence = TMPL_ATTR_LIST_ALLOW,
 			.bare_word_enum = v3_compat,
-		}
+		},
+		.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 	};
 
 	while (true) {
diff --git a/src/lib/server/virtual_servers.c b/src/lib/server/virtual_servers.c
index 4cf8d2fc1cb..578f0203c28 100644
--- a/src/lib/server/virtual_servers.c
+++ b/src/lib/server/virtual_servers.c
@@ -1619,6 +1619,8 @@ int virtual_servers_instantiate(void)
 					.dict_def = dict,
 					.list_def = request_attr_request,
 				},
+
+				.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 			};
 
 			fr_assert(parse_rules.attr.dict_def != NULL);
diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c
index 0e9f9fc023a..38574f58fb0 100644
--- a/src/lib/unlang/xlat_builtin.c
+++ b/src/lib/unlang/xlat_builtin.c
@@ -758,6 +758,7 @@ static xlat_action_t xlat_func_taint(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out,
 
 		while ((child = fr_value_box_list_pop_head(&vb->vb_group)) != NULL) {
 			child->tainted = true;
+			fr_value_box_mark_unsafe(child);
 
 			fr_dcursor_append(out, child);
 		}
diff --git a/src/lib/util/value.c b/src/lib/util/value.c
index a71d38778ad..0b380a198e5 100644
--- a/src/lib/util/value.c
+++ b/src/lib/util/value.c
@@ -6358,6 +6358,16 @@ void fr_value_box_list_verify(char const *file, int line, fr_value_box_list_t co
  */
 void _fr_value_box_mark_safe_for(fr_value_box_t *vb, fr_value_box_safe_for_t safe_for)
 {
+	/*
+	 *	Don't over-ride value-boxes which are already safe, unless we want to mark them as being
+	 *	completely unsafe.
+	 */
+	if ((vb->safe_for == FR_VALUE_BOX_SAFE_FOR_ANY) &&
+	    (safe_for != FR_VALUE_BOX_SAFE_FOR_NONE)) {
+		fr_assert(!vb->tainted);
+		return;
+	}
+
 	vb->safe_for = safe_for;
 }
 
@@ -6367,7 +6377,7 @@ void _fr_value_box_mark_safe_for(fr_value_box_t *vb, fr_value_box_safe_for_t saf
  */
 void fr_value_box_mark_unsafe(fr_value_box_t *vb)
 {
-	vb->safe_for = 0;
+	vb->safe_for = FR_VALUE_BOX_SAFE_FOR_NONE;
 }
 
 /** Set the escaped flag for all value boxes in a list
@@ -6379,7 +6389,17 @@ void fr_value_box_mark_unsafe(fr_value_box_t *vb)
  */
 void fr_value_box_list_mark_safe_for(fr_value_box_list_t *list, fr_value_box_safe_for_t safe_for)
 {
-	fr_value_box_list_foreach(list, vb) vb->safe_for = safe_for;
+	fr_value_box_list_foreach(list, vb) {
+		/*
+		 *	Don't over-ride value-boxes which are already safe.
+		 */
+		if (vb->safe_for == FR_VALUE_BOX_SAFE_FOR_ANY) {
+			fr_assert(!vb->tainted);
+
+		} else {
+			vb->safe_for = safe_for;
+		}
+	}
 }
 
 /** Check truthiness of values.
diff --git a/src/lib/util/value.h b/src/lib/util/value.h
index 208084ca8f3..0bdc37cb242 100644
--- a/src/lib/util/value.h
+++ b/src/lib/util/value.h
@@ -154,6 +154,17 @@ typedef union {
  */
 typedef uintptr_t fr_value_box_safe_for_t;
 
+/*
+ *	The default value of "completely unsafe" is zero.  That way any initialization routines will default
+ *	to marking the data as unsafe.
+ *
+ *	The only data which should be marked as being completely safe is data taken from the configuration
+ *	files which are managed by the administrator.  Data create by end users (e.g. passwords) should always
+ *	be marked as unsafe.
+ */
+#define FR_VALUE_BOX_SAFE_FOR_NONE ((uintptr_t) 0)
+#define FR_VALUE_BOX_SAFE_FOR_ANY (~((uintptr_t) 0))
+
 /** Union containing all data types supported by the server
  *
  * This union contains all data types that can be represented by fr_pair_ts. It may also be used in other parts
@@ -1052,7 +1063,7 @@ void		_fr_value_box_mark_safe_for(fr_value_box_t *box, fr_value_box_safe_for_t s
 void		fr_value_box_mark_unsafe(fr_value_box_t *box)
 		CC_HINT(nonnull);
 
-#define		fr_value_box_is_safe_for(_box, _safe_for) (_box->safe_for == (fr_value_box_safe_for_t)_safe_for)
+#define		fr_value_box_is_safe_for(_box, _safe_for) ((_box->safe_for == (fr_value_box_safe_for_t)_safe_for) || (_box->safe_for == FR_VALUE_BOX_SAFE_FOR_ANY))
 
 void		fr_value_box_list_mark_safe_for(fr_value_box_list_t *list, fr_value_box_safe_for_t safe_for);
 
diff --git a/src/modules/rlm_linelog/rlm_linelog.c b/src/modules/rlm_linelog/rlm_linelog.c
index 8c14898f0db..212b45cc154 100644
--- a/src/modules/rlm_linelog/rlm_linelog.c
+++ b/src/modules/rlm_linelog/rlm_linelog.c
@@ -767,7 +767,8 @@ static unlang_action_t CC_HINT(nonnull) mod_do_linelog(rlm_rcode_t *p_result, mo
 						.xlat = {
 							.runtime_el = unlang_interpret_event_list(request),
 						},
-					 	.at_runtime = true
+						.at_runtime = true,
+						.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 					 });
 		if (!vpt) {
 			REMARKER(tmpl_str, -slen, "%s", fr_strerror());
diff --git a/src/modules/rlm_radius/rlm_radius.c b/src/modules/rlm_radius/rlm_radius.c
index 2176b3a9d08..fa7bd460d17 100644
--- a/src/modules/rlm_radius/rlm_radius.c
+++ b/src/modules/rlm_radius/rlm_radius.c
@@ -498,7 +498,8 @@ static int status_check_update_parse(TALLOC_CTX *ctx, void *out, void *parent,
 				.list_def = request_attr_request,
 				.list_presence = TMPL_ATTR_LIST_FORBID,
 				.prefix = TMPL_ATTR_REF_PREFIX_AUTO,
-			}
+			},
+			.literals_safe_for = FR_VALUE_BOX_SAFE_FOR_ANY,
 		};
 
 		rcode = map_afrom_cs(ctx, head, cs, &parse_rules, &parse_rules, status_check_verify, parent, 128);
diff --git a/src/tests/keywords/regex-escape b/src/tests/keywords/regex-escape.ignore
similarity index 100%
rename from src/tests/keywords/regex-escape
rename to src/tests/keywords/regex-escape.ignore
diff --git a/src/tests/keywords/regex-lhs b/src/tests/keywords/regex-lhs
index d52a3a6ea09..179589ec1d1 100644
--- a/src/tests/keywords/regex-lhs
+++ b/src/tests/keywords/regex-lhs
@@ -1,5 +1,5 @@
 #
-#  PRE: if regex-escape
+#  PRE: if
 #
 string test_string1
 string test_string2