From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 26 Mar 2025 20:56:32 +0000 (-0400)
Subject: mark unsafe attributes as SAFE_FOR_NONE
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f04553ce4b8199a0b68fd969532fce92150b136a;p=thirdparty%2Ffreeradius-server.git

mark unsafe attributes as SAFE_FOR_NONE

no matter where they come from.  This catches SQL and the files
module.  The usersfile code sets SAFE_FOR_ANY, but that is then
over-ridden with da->flags.unsafe.  If we later want to make the
SQL module default to SAFE_FOR_ANY, we can make that change
without affecting the security of Cleartext-Password

Assert that creating pairs from random strings always results in
the output value box being unsafe for anything
---

diff --git a/src/lib/server/pairmove.c b/src/lib/server/pairmove.c
index 4268c4bfe67..86477411c62 100644
--- a/src/lib/server/pairmove.c
+++ b/src/lib/server/pairmove.c
@@ -727,6 +727,7 @@ int radius_legacy_map_apply(request_t *request, map_t const *map, fr_edit_list_t
 			return -1;
 		}
 
+		if (vp->da->flags.unsafe) fr_value_box_mark_unsafe(&vp->data);
 		TALLOC_FREE(to_free);
 		return 0;
 	}
diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index a3f94691bd6..423f3afb447 100644
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -797,6 +797,7 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 		 */
 		if (fr_value_box_cast(vp, &vp->data, vp->vp_type, vp->da, box) < 0) goto fail;
 		vp->op = T_OP_EQ;
+		if (vp->da->flags.unsafe) fr_value_box_mark_unsafe(&vp->data);
 
 		if (single) goto done;
 
@@ -823,6 +824,7 @@ static int apply_edits_to_leaf(request_t *request, unlang_frame_state_edit_t *st
 
 			MEM(vp = fr_pair_afrom_da(current->lhs.vp_parent, da));
 			if (fr_value_box_cast(vp, &vp->data, vp->vp_type, vp->da, box) < 0) goto fail;
+			if (vp->da->flags.unsafe) fr_value_box_mark_unsafe(&vp->data);
 
 			if (fr_edit_list_insert_pair_tail(state->el, &current->lhs.vp_parent->vp_group, vp) < 0) goto fail;
 			vp->op = T_OP_EQ;
@@ -850,6 +852,7 @@ apply_op:
 	 */
 	while (box) {
 		RDEBUG_ASSIGN(current->lhs.vpt->name, map->op, box);
+		if (current->lhs.vp->da->flags.unsafe) fr_value_box_mark_unsafe(box);
 
 		/*
 		 *	The apply function also takes care of doing data type upcasting and conversion.  So we don't
diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index a7b2238ab54..c338f8f96f6 100644
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -2617,6 +2617,8 @@ int fr_pair_value_from_str(fr_pair_t *vp, char const *value, size_t inlen,
 				  uerules,
 				  tainted) < 0) return -1;
 
+	fr_assert(vp->data.safe_for == FR_VALUE_BOX_SAFE_FOR_NONE);
+
 	PAIR_VERIFY(vp);
 
 	return 0;