From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 12 May 2025 16:33:40 +0000 (-0400)
Subject: if it's not a leaf type, ensure it's a structural one
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=88cc3a39f2d1eb8cc07d507a93b705cbd8e87453;p=thirdparty%2Ffreeradius-server.git

if it's not a leaf type, ensure it's a structural one

so that the code behaves sanely or at least asserts, if passed
another type
---

diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c
index 9807d304e80..6c697c62c6d 100644
--- a/src/bin/unit_test_module.c
+++ b/src/bin/unit_test_module.c
@@ -124,6 +124,8 @@ static void pair_mutable(fr_pair_t *vp)
 		return;
 	}
 
+	fr_assert(fr_type_is_structural(vp->vp_type));
+
 	fr_pair_list_foreach(&vp->vp_group, child) {
 		pair_mutable(child);
 	}
diff --git a/src/lib/server/pairmove.c b/src/lib/server/pairmove.c
index 0a9b4cc39b9..4b6b4dcee4c 100644
--- a/src/lib/server/pairmove.c
+++ b/src/lib/server/pairmove.c
@@ -562,6 +562,8 @@ int radius_legacy_map_apply(request_t *request, map_t const *map, fr_edit_list_t
 			if (fr_type_is_leaf(vp->vp_type)) {
 				if (fr_edit_list_save_pair_value(el, vp) < 0) return -1;
 			} else {
+				fr_assert(fr_type_is_structural(vp->vp_type));
+
 				if (fr_edit_list_free_pair_children(el, vp) < 0) return -1;
 			}
 			break;
diff --git a/src/lib/unlang/edit.c b/src/lib/unlang/edit.c
index 61d550a7480..caf694c869f 100644
--- a/src/lib/unlang/edit.c
+++ b/src/lib/unlang/edit.c
@@ -1031,6 +1031,8 @@ static int check_rhs(request_t *request, unlang_frame_state_edit_t *state, edit_
 	if (fr_type_is_leaf(tmpl_attr_tail_da(current->lhs.vpt)->type)) {
 		if (apply_edits_to_leaf(request, state, current) < 0) return -1;
 	} else {
+		fr_assert(fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type));
+
 		if (apply_edits_to_list(request, state, current) < 0) return -1;
 	}
 
@@ -1093,6 +1095,8 @@ static int expand_rhs_list(request_t *request, unlang_frame_state_edit_t *state,
 		child->check_lhs = check_lhs_value;
 		child->expanded_lhs = expanded_lhs_value;
 	} else {
+		fr_assert(fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type));
+
 		child->ctx = current->lhs.vp ? (TALLOC_CTX *) current->lhs.vp : (TALLOC_CTX *) child;
 		child->check_lhs = check_lhs_nested;
 		child->expanded_lhs = expanded_lhs_attribute;
@@ -1312,6 +1316,8 @@ static int check_lhs_nested(request_t *request, unlang_frame_state_edit_t *state
 		return expand_rhs(request, state, current);
 	}
 
+	fr_assert(fr_type_is_structural(tmpl_attr_tail_da(current->lhs.vpt)->type));
+
 	/*
 	 *	We have a parent, so we know that attribute exist.  Which means that we don't need to call a
 	 *	cursor function to create this VP.
diff --git a/src/lib/unlang/foreach.c b/src/lib/unlang/foreach.c
index 3a05a8973e5..229a679f8a4 100644
--- a/src/lib/unlang/foreach.c
+++ b/src/lib/unlang/foreach.c
@@ -117,6 +117,8 @@ static int unlang_foreach_pair_copy(fr_pair_t *to, fr_pair_t *from, fr_dict_attr
 			continue;
 		}
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		if (unlang_foreach_pair_copy(child, vp, vp->da) < 0) return -1;
 	}
 
@@ -313,6 +315,8 @@ static unlang_action_t unlang_foreach_attr_next(rlm_rcode_t *p_result, request_t
 			 */
 		}
 	} else {
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		/*
 		 *	@todo - copy the pairs back?
 		 */
diff --git a/src/lib/util/cbor.c b/src/lib/util/cbor.c
index 61e6d2d9ab4..b6543428c29 100644
--- a/src/lib/util/cbor.c
+++ b/src/lib/util/cbor.c
@@ -1715,6 +1715,8 @@ ssize_t fr_cbor_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dbuff_t *db
 		goto done;
 	}
 
+	fr_assert(fr_type_is_structural(da->type));
+
 	switch (da->type) {
 		/*
 		 *	All of these are essentially the same.
diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
index 949d983b66a..a0bc5d03bcc 100644
--- a/src/lib/util/pair.c
+++ b/src/lib/util/pair.c
@@ -208,6 +208,8 @@ static inline CC_HINT(always_inline) void pair_init_from_da(fr_pair_t *vp, fr_di
 		memset(&vp->data, 0xff, sizeof(vp->data));
 #endif
 
+		fr_assert(fr_type_is_structural(da->type));
+
 		/*
 		 *	Make sure that the pad field is initialized.
 		 */
@@ -327,7 +329,7 @@ int fr_pair_reinit_from_da(fr_pair_list_t *list, fr_pair_t *vp, fr_dict_attr_t c
 
 		if ((da->type != vp->vp_type) && (fr_value_box_cast_in_place(vp, &vp->data, da->type, da) < 0)) return -1;
 	} else {
-		fr_assert(fr_type_is_leaf(vp->vp_type) || (fr_pair_list_num_elements(&vp->vp_group) == 0));
+		fr_assert(fr_type_is_leaf(vp->vp_type) || (fr_type_is_structural(vp->vp_type) && (fr_pair_list_num_elements(&vp->vp_group) == 0)));
 
 		fr_value_box_init(&vp->data, da->type, da, false);
 	}
@@ -1052,6 +1054,8 @@ fr_pair_t *fr_pair_list_iter_leaf(fr_pair_list_t *list, fr_pair_t *vp)
 	next_sibling:
 		if (fr_type_is_leaf(vp->vp_type)) return vp;
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		vp = fr_pair_list_iter_leaf(&vp->vp_group, NULL);
 		if (vp) return vp;
 
@@ -2283,6 +2287,8 @@ bool fr_pair_immutable(fr_pair_t const *vp)
 {
 	if (fr_type_is_leaf(vp->vp_type)) return vp->vp_immutable;
 
+	fr_assert(fr_type_is_structural(vp->vp_type));
+
 	fr_pair_list_foreach(&vp->vp_group, child) {
 		if (fr_type_is_leaf(child->vp_type)) {
 			if (child->vp_immutable) return true;
@@ -2290,6 +2296,8 @@ bool fr_pair_immutable(fr_pair_t const *vp)
 			continue;
 		}
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		if (fr_pair_immutable(child)) return true;
 	}
 
@@ -3510,6 +3518,8 @@ static void fprintf_pair_list(FILE *fp, fr_pair_list_t const *list, int depth)
 			continue;
 		}
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		fprintf(fp, "%s = {\n", vp->da->name);
 		fprintf_pair_list(fp, &vp->vp_group, depth + 1);
 		fprintf(fp, "%.*s}\n", depth, spaces);
@@ -3531,6 +3541,8 @@ void fr_fprintf_pair(FILE *fp, char const *msg, fr_pair_t const *vp)
 	if (fr_type_is_leaf(vp->vp_type)) {
 		fr_fprintf(fp, "%s %s %pV\n", vp->da->name, fr_tokens[vp->op], &vp->data);
 	} else {
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		fprintf(fp, "%s = {\n", vp->da->name);
 		fprintf_pair_list(fp, &vp->vp_group, 1);
 		fprintf(fp, "}\n");
diff --git a/src/lib/util/pair_print.c b/src/lib/util/pair_print.c
index abcf197ef5b..b95db80a540 100644
--- a/src/lib/util/pair_print.c
+++ b/src/lib/util/pair_print.c
@@ -215,6 +215,8 @@ ssize_t fr_pair_print_secure(fr_sbuff_t *out, fr_dict_attr_t const *parent, fr_p
 		fr_pair_t *child;
 		fr_dcursor_t cursor;
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		FR_SBUFF_IN_CHAR_RETURN(&our_out, '{', ' ');
 		for (child = fr_pair_dcursor_init(&cursor, &vp->vp_group);
 		     child != NULL;
diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c
index d3b2c0e86c4..2892af8d3c4 100644
--- a/src/modules/rlm_detail/rlm_detail.c
+++ b/src/modules/rlm_detail/rlm_detail.c
@@ -192,6 +192,8 @@ static int detail_recurse(FILE *out, fr_hash_table_t *ht, fr_pair_list_t *list)
 			continue;
 		}
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		if (detail_recurse(out, ht, &vp->vp_group) < 0) return -1;
 	}
 
@@ -287,6 +289,8 @@ static int detail_write(FILE *out, rlm_detail_t const *inst, request_t *request,
 			continue;
 		}
 
+		fr_assert(fr_type_is_structural(vp->vp_type));
+
 		if (detail_recurse(out, ht, &vp->vp_group) < 0) goto fail;
 	}
 
diff --git a/src/modules/rlm_lua/lua.c b/src/modules/rlm_lua/lua.c
index c97548eea83..512484de894 100644
--- a/src/modules/rlm_lua/lua.c
+++ b/src/modules/rlm_lua/lua.c
@@ -601,6 +601,8 @@ static int _lua_pair_accessor(lua_State *L)
 			return 1;
 		}
 
+		fr_assert(fr_type_is_structural(pair_data->da->type));
+
 		/*
 		 *	Retrieving a structural attribute returns a new table.
 		 */
diff --git a/src/modules/rlm_python/rlm_python.c b/src/modules/rlm_python/rlm_python.c
index abcd3bea35a..ac02e4a4c01 100644
--- a/src/modules/rlm_python/rlm_python.c
+++ b/src/modules/rlm_python/rlm_python.c
@@ -534,8 +534,12 @@ static PyObject *py_freeradius_attribute_instance(PyObject *self, PyObject *attr
 
 	if (fr_type_is_leaf(init_pair->da->type)) {
 		pair = PyObject_New(py_freeradius_pair_t, (PyTypeObject *)&py_freeradius_value_pair_def);
-	} else {
+
+	} else if (fr_type_is_struct(init_pair->da->type)) {
 		pair = PyObject_New(py_freeradius_pair_t, (PyTypeObject *)&py_freeradius_grouping_pair_def);
+	} else {
+		PyErr_SetString(PyExc_AttributeError, "Unsupported data type");
+		return NULL;
 	}
 	if (!pair) {
 		PyErr_SetString(PyExc_MemoryError, "Failed to allocate PyObject");
@@ -594,8 +598,11 @@ static PyObject *py_freeradius_pair_map_subscript(PyObject *self, PyObject *attr
 
 	if (fr_type_is_leaf(da->type)) {
 		pair = PyObject_New(py_freeradius_pair_t, (PyTypeObject *)&py_freeradius_value_pair_def);
-	} else {
+	} else if (fr_type_is_structural(da->type)) {
 		pair = PyObject_New(py_freeradius_pair_t, (PyTypeObject *)&py_freeradius_grouping_pair_def);
+	} else {
+		PyErr_SetString(PyExc_AttributeError, "Unsupported data type");
+		return NULL;
 	}
 	if (!pair) {
 		PyErr_SetString(PyExc_MemoryError, "Failed to allocate PyObject");