]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
clean up use of safe_for
authorAlan T. DeKok <aland@freeradius.org>
Mon, 10 Mar 2025 22:03:43 +0000 (18:03 -0400)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 12 Mar 2025 07:07:07 +0000 (09:07 +0200)
remove explicit safe_for argument from xlat_tokenize().  It is
already being passed in the tmpl_rules_t.   Passing a separate
and explicit 0 breaks all kinds of things.  e.g.. "..." gets
marked correctly, but %{"..."} does not.

require xlat_tokenize() to be passed a tmpl_rules_t.

update trigger code to allow the use of attributes from the
local dictionary.  Which is usually internal, but should arguably
also be allowed to be a protocol dictionary.  So that we can send
protocol-specfic triggers

add commented out function to double-check safe_for after parsing.

update callers of xlat_tokenize() to create a local tmpl_rules_t,
and pass the correct safe_for

src/bin/unit_test_attribute.c
src/bin/unit_test_module.c
src/lib/server/cf_parse.c
src/lib/server/tmpl_tokenize.c
src/lib/server/trigger.c
src/lib/unlang/xlat.h
src/lib/unlang/xlat_eval.c
src/lib/unlang/xlat_tokenize.c
src/modules/rlm_sqlcounter/rlm_sqlcounter.c

index 1a2093af298aa4e158abdd2a7e11ab9487c1e938..23e38683f2304f44f894bf80d3d8874ec80228dd 100644 (file)
@@ -2864,8 +2864,7 @@ static size_t command_xlat_normalise(command_result_t *result, command_file_ctx_
                                                .allow_unresolved = cc->tmpl_rules.attr.allow_unresolved
                                        },
                                        .xlat = cc->tmpl_rules.xlat,
-                               },
-                               0);
+                               });
        if (dec_len <= 0) {
                fr_strerror_printf_push_head("ERROR offset %d", (int) -dec_len);
 
index 77df6d1b75938de60756e83fea402f4745cf737f..0fef6f6be586fb4a878c9a8f45e2cffb781cf368 100644 (file)
@@ -486,7 +486,7 @@ static bool do_xlats(fr_event_list_t *el, request_t *request, char const *filena
                                                                };
 
 
-                       slen = xlat_tokenize(xlat_ctx, &head, &line, &p_rules, &t_rules, 0);
+                       slen = xlat_tokenize(xlat_ctx, &head, &line, &p_rules, &t_rules);
                        if (slen <= 0) {
                                talloc_free(xlat_ctx);
                                fr_sbuff_in_sprintf(&out, "ERROR offset %d '%s'", (int) -slen, fr_strerror());
index de85876c9784b660d40a216943378d3525054ac1..89d5641545a5f501daaf496f288dd544021302c1 100644 (file)
@@ -1402,7 +1402,7 @@ int cf_section_parse_pass2(void *base, CONF_SECTION *cs)
                                                             .allow_unresolved = false,
                                                             .allow_foreign = (dict == NULL)
                                                     },
-                                            }, 0);
+                                            });
                        if (slen < 0) {
                                char *spaces, *text;
 
index 9579bee4f2a077eded99bd86c13ded0dc175713b..f749ef5c05c5a4a33b7e10209f906c0e5f807fcb 100644 (file)
@@ -3262,7 +3262,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
                        xlat_exp_head_t *head = NULL;
 
                        vpt = tmpl_alloc_null(ctx);
-                       slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, t_rules->literals_safe_for);
+                       slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules);
                        if (slen <= 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
                        if (xlat_needs_resolving(head)) UNRESOLVED_SET(&type);
@@ -3471,7 +3471,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 
                vpt = tmpl_alloc_null(ctx);
 
-               slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, t_rules->literals_safe_for);
+               slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules);
                if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
                /*
@@ -3555,6 +3555,9 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
        {
                xlat_exp_head_t         *head = NULL;
                tmpl_type_t             type = TMPL_TYPE_REGEX_XLAT;
+               tmpl_rules_t            arg_t_rules = *t_rules;
+
+               arg_t_rules.literals_safe_for = FR_REGEX_SAFE_FOR;
 
                if (!fr_type_is_null(t_rules->cast)) {
                        fr_strerror_const("Casts cannot be used with regular expressions");
@@ -3564,7 +3567,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,
 
                vpt = tmpl_alloc_null(ctx);
 
-               slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, FR_REGEX_SAFE_FOR);
+               slen = xlat_tokenize(vpt, &head, &our_in, p_rules, &arg_t_rules);
                if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in);
 
                /*
@@ -4003,6 +4006,7 @@ int tmpl_cast_in_place(tmpl_t *vpt, fr_type_t type, fr_dict_attr_t const *enumv)
                 *      i.e. TMPL_TYPE_DATA_UNRESOLVED != TMPL_TYPE_DATA(FR_TYPE_STRING)
                 */
                if (fr_value_box_cast_in_place(vpt, &vpt->data.literal, type, NULL) < 0) return -1;
+//             fr_value_box_mark_safe_for(&vpt->data.literal, vpt->rules.literals_safe_for); ??? is this necessary?
 
                /*
                 *      Strings get quoted, everything else is a bare
index 9d32f04898ca3ab7d88c0d0bfc795bee9392e10b..c8ce80e625ac235fe005e8bd2bc838cc01d30da7 100644 (file)
@@ -380,11 +380,17 @@ int trigger_exec(unlang_interpret_t *intp,
        MEM(trigger = talloc_zero(request, fr_trigger_t));
        fr_value_box_list_init(&trigger->args);
        trigger->command = talloc_strdup(trigger, value);
-       trigger->timeout = fr_time_delta_from_sec(5);   /* FIXME - Should be configurable? */
+       trigger->timeout = fr_time_delta_from_sec(5);   /* @todo - Should be configurable? */
 
        slen = xlat_tokenize_argv(trigger, &trigger->xlat,
                                  &FR_SBUFF_IN(trigger->command, talloc_array_length(trigger->command) - 1),
-                                 NULL, NULL, NULL, true);
+                                 NULL, NULL, &(tmpl_rules_t) {
+                                         .attr = {
+                                                 .dict_def = request->dict,
+                                                 .list_def = request_attr_request,
+                                                 .allow_unresolved = false,
+                                         },
+                                 }, true);
        if (slen <= 0) {
                char *spaces, *text;
 
index 6bf48249374064976b6fb4769a8edbb69a9f2bf9..6274e15380557cca0861929d2bf60573b357d2e6 100644 (file)
@@ -405,12 +405,11 @@ fr_slen_t xlat_tokenize_condition(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sb
                                        fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules);
 
 fr_slen_t      xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sbuff_t *in,
-                                  xlat_t const *xlat,
-                                  fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules, bool spaces);
+                                  xlat_t const *xlat, fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
+                                  bool spaces) CC_HINT(nonnull(1,2,3,6));
 
 fr_slen_t      xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **head, fr_sbuff_t *in,
-                             fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-                             fr_value_box_safe_for_t literals_safe_for);
+                             fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules);
 
 fr_slen_t      xlat_print(fr_sbuff_t *in, xlat_exp_head_t const *node, fr_sbuff_escape_rules_t const *e_rules);
 
index f226552ef1d8621b6cdf56cc4decd8cce2070140..6c1ac030e1cd83bc33ac9344da5f5d4bd7099cdf 100644 (file)
@@ -1509,7 +1509,7 @@ ssize_t _xlat_eval(TALLOC_CTX *ctx, char **out, size_t outlen, request_t *reques
                                            .runtime_el = unlang_interpret_event_list(request),
                                    },
                                    .at_runtime = true,
-                           }, 0);
+                           });
        if (len == 0) {
                if (*out) {
                        **out = '\0';
index eb4d163338e83a76a0c10ac882e36f67f086cf73..921cd3983c6019de8a9c0c6e03332179b5231482 100644 (file)
@@ -220,9 +220,9 @@ static int xlat_validate_function_arg(xlat_arg_parser_t const *arg_p, xlat_exp_t
                        fr_assert(tmpl_rules_cast(vpt) == FR_TYPE_NULL);
 
                        fr_value_box_steal(node, &node->data, tmpl_value(vpt));
-
                        talloc_free(vpt);
                        xlat_exp_set_type(node, XLAT_BOX);
+                       fr_value_box_mark_safe_for(&node->data, arg_p->safe_for);
 
                } else {
                        fr_assert(!tmpl_is_data_unresolved(node->vpt));
@@ -310,7 +310,7 @@ fr_slen_t xlat_validate_function_args(xlat_exp_t *node)
  *     - 0 if the string was parsed into a function.
  *     - <0 on parse error.
  */
-static int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tmpl_rules_t const *t_rules)
+static CC_HINT(nonnull) int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tmpl_rules_t const *t_rules)
 {
        char c;
        xlat_exp_t *node;
@@ -403,7 +403,7 @@ static int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tm
         */
        node = xlat_exp_alloc(head, XLAT_FUNC, fr_sbuff_current(&m_s), fr_sbuff_behind(&m_s));
        if (!func) {
-               if (!t_rules || !t_rules->attr.allow_unresolved|| t_rules->at_runtime) {
+               if (!t_rules->attr.allow_unresolved|| t_rules->at_runtime) {
                        fr_strerror_const("Unresolved expansion functions are not allowed here");
                        fr_sbuff_set(in, &m_s);         /* backtrack */
                        fr_sbuff_marker_release(&m_s);
@@ -413,7 +413,7 @@ static int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tm
                node->flags.needs_resolving = true;     /* Needs resolution during pass2 */
        } else {
                node->call.func = func;
-               if (t_rules) node->call.dict = t_rules->attr.dict_def;
+               node->call.dict = t_rules->attr.dict_def;
                node->flags = func->flags;
                node->flags.impure_func = !func->flags.pure;
                node->call.input_type = func->input_type;
@@ -425,7 +425,7 @@ static int xlat_tokenize_function_args(xlat_exp_head_t *head, fr_sbuff_t *in, tm
         *      The caller might want the _output_ cast to something.  But that doesn't mean we cast each
         *      _argument_ to the xlat function.
         */
-       if (t_rules && (t_rules->cast != FR_TYPE_NULL)) {
+       if (t_rules->cast != FR_TYPE_NULL) {
                my_t_rules = *t_rules;
                my_t_rules.cast = FR_TYPE_NULL;
                t_rules = &my_t_rules;
@@ -493,8 +493,8 @@ static int xlat_resolve_virtual_attribute(xlat_exp_t *node, tmpl_t *vpt)
 /** Parse an attribute ref or a virtual attribute
  *
  */
-static ssize_t xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
-                                      fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules, tmpl_attr_prefix_t attr_prefix)
+static CC_HINT(nonnull(1,2,4)) ssize_t xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
+                                                              fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules, tmpl_attr_prefix_t attr_prefix)
 {
        tmpl_attr_error_t       err;
        tmpl_t                  *vpt = NULL;
@@ -520,12 +520,7 @@ static ssize_t xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
         *      and instead are "virtual" attributes like
         *      Foreach-Variable-N.
         */
-       if (t_rules) {
-               memset(&our_t_rules, 0, sizeof(our_t_rules));
-               our_t_rules = *t_rules;
-       } else {
-               memset(&our_t_rules, 0, sizeof(our_t_rules));
-       }
+       our_t_rules = *t_rules;
 
        our_t_rules.attr.allow_unresolved = true;               /* So we can check for virtual attributes later */
 
@@ -564,7 +559,7 @@ static ssize_t xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in,
                 */
                if ((tmpl_attr_num_elements(vpt) == 2) && (xlat_resolve_virtual_attribute(node, vpt) == 0)) goto done;
 
-               if (!t_rules || !t_rules->attr.allow_unresolved) {
+               if (!t_rules->attr.allow_unresolved) {
                        talloc_free(vpt);
 
                        fr_strerror_const("Unresolved attributes not allowed in expansions here");
@@ -824,15 +819,12 @@ check_for_attr:
  * @param[in] in               sbuff to parse.
  * @param[in] p_rules          that control parsing.
  * @param[in] t_rules          that control attribute reference and xlat function parsing.
- * @param[in] safe_for         mark up literal values as being pre-escaped.  May be merged
- *                             with t_rules in future.
  * @return
  *     - <0 on failure
  *     - >=0 for number of bytes parsed
  */
-static ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
-                                  fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-                                  fr_value_box_safe_for_t safe_for)
+static CC_HINT(nonnull(1,2,4)) ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
+                                                          fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules)
 {
        xlat_exp_t                      *node = NULL;
        fr_slen_t                       slen;
@@ -883,7 +875,7 @@ static ssize_t xlat_tokenize_input(xlat_exp_head_t *head, fr_sbuff_t *in,
                do_value_box:
                        xlat_exp_set_name_buffer_shallow(node, str);
                        fr_value_box_strdup(node, &node->data, NULL, str, false);
-                       fr_value_box_mark_safe_for(&node->data, safe_for);
+                       fr_value_box_mark_safe_for(&node->data, t_rules->literals_safe_for);
                        node->flags.constant = true;
                        fr_assert(node->flags.pure);
 
@@ -1348,6 +1340,35 @@ ssize_t xlat_print(fr_sbuff_t *out, xlat_exp_head_t const *head, fr_sbuff_escape
        return fr_sbuff_used_total(out) - at_in;
 }
 
+#if 0
+static void xlat_safe_for(xlat_exp_head_t *head, fr_value_box_safe_for_t safe_for)
+{
+       xlat_exp_foreach(head, node) {
+               switch (node->type) {
+               case XLAT_BOX:
+                       if (node->data.safe_for != safe_for) {
+                               ERROR("FAILED %lx %lx - %s", node->data.safe_for, safe_for, node->fmt);
+                       }
+                       fr_assert(node->data.safe_for == safe_for);
+                       break;
+
+               case XLAT_GROUP:
+                       xlat_safe_for(node->group, safe_for);
+                       break;
+
+               case XLAT_TMPL:
+                       if (!tmpl_is_xlat(node->vpt)) break;
+
+                       xlat_safe_for(tmpl_xlat(node->vpt), safe_for);
+                       break;
+
+               default:
+                       break;
+               }
+       }
+}
+#endif
+
 
 /** Tokenize an xlat expansion into a series of XLAT_TYPE_CHILD arguments
  *
@@ -1378,8 +1399,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
        fr_sbuff_parse_rules_t          tmp_p_rules;
        xlat_exp_head_t                 *head;
        xlat_arg_parser_t const         *arg = NULL, *arg_start;
-       tmpl_rules_t const              *our_t_rules = t_rules;
-       tmpl_rules_t                    tmp_t_rules;
+       tmpl_rules_t                    arg_t_rules;
 
        if (xlat && xlat->args) {
                arg_start = arg = xlat->args;   /* Track the arguments as we parse */
@@ -1388,6 +1408,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                                                                  XLAT_ARG_PARSER_TERMINATOR };
                arg_start = arg = &default_arg[0];
        }
+       arg_t_rules = *t_rules;
 
        if (spaces) {
                fr_assert(p_rules != &xlat_function_arg_rules);
@@ -1421,15 +1442,12 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                 *      expression to be just about anything.
                 */
                if (!xlat_func_bare_words) {
-                       tmp_t_rules = *t_rules;
-                       our_t_rules = &tmp_t_rules;
-
-                       tmp_t_rules.enumv = NULL;
-                       tmp_t_rules.cast = FR_TYPE_NULL;
-                       tmp_t_rules.attr.namespace = NULL;
-                       tmp_t_rules.attr.request_def = NULL;
-                       tmp_t_rules.attr.list_def = request_attr_request;
-                       tmp_t_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW;
+                       arg_t_rules.enumv = NULL;
+                       arg_t_rules.cast = FR_TYPE_NULL;
+                       arg_t_rules.attr.namespace = NULL;
+                       arg_t_rules.attr.request_def = NULL;
+                       arg_t_rules.attr.list_def = request_attr_request;
+                       arg_t_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW;
                }
        }
 
@@ -1449,6 +1467,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                size_t          len;
 
                fr_sbuff_set(&m, &our_in);      /* Record start of argument */
+               arg_t_rules.literals_safe_for = arg->safe_for;
 
                /*
                 *      Whitespace isn't significant for comma-separated argvs
@@ -1497,7 +1516,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                                 *
                                 *      No spaces - each arugment is an expression, which can have embedded spaces.
                                 */
-                               slen = xlat_tokenize_input(node->group, &our_in, our_p_rules, t_rules, arg->safe_for);
+                               slen = xlat_tokenize_input(node->group, &our_in, our_p_rules, &arg_t_rules);
 
                        } else {
                        tokenize_expression:
@@ -1508,7 +1527,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                                        slen = 0;
 
                                } else {
-                                       slen = xlat_tokenize_expression(node, &node->group, &our_in, our_p_rules, our_t_rules);
+                                       slen = xlat_tokenize_expression(node, &node->group, &our_in, our_p_rules, &arg_t_rules);
                                }
                        }
                        if (slen < 0) {
@@ -1551,7 +1570,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                        XLAT_DEBUG("ARGV double quotes <-- %.*s", (int) fr_sbuff_remaining(&our_in), fr_sbuff_current(&our_in));
 
                        if (xlat_tokenize_input(node->group, &our_in,
-                                               &value_parse_rules_double_quoted, t_rules, arg->safe_for) < 0) goto error;
+                                               &value_parse_rules_double_quoted, &arg_t_rules) < 0) goto error;
                        break;
 
                /*
@@ -1600,6 +1619,13 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
                fmt = talloc_bstrndup(node, fr_sbuff_current(&m), fr_sbuff_behind(&m));
                xlat_exp_set_name_buffer_shallow(node, fmt);
 
+               /*
+                *      Assert that the parser has created things which are safe for the current argument.
+                *
+                *      @todo - function should be marked up with safe_for, and not each individual argument.
+                */
+//             xlat_safe_for(node->group, arg->safe_for);
+
                node->flags = node->group->flags;
 
                xlat_exp_insert_tail(head, node);
@@ -1682,11 +1708,6 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
  * @param[in] p_rules                  controlling how the string containing the xlat
  *                                     expansions should be parsed.
  * @param[in] t_rules                  controlling how attribute references are parsed.
- *                                     Do NOT alter this function to take tmpl_rules_t
- *                                     as this provides another value for literals_safe_for
- *                                     and this gets very confusing.
- * @param[in] literals_safe_for                the safe_for value to assign to any literals occurring at the
- *                                     top level of the expansion.
  * @return
  *     - >0 on success.
  *     - 0 and *head == NULL - Parse failure on first char.
@@ -1694,8 +1715,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t
  *     - < 0 the negative offset of the parse failure.
  */
 fr_slen_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t *in,
-                       fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules,
-                       fr_value_box_safe_for_t literals_safe_for)
+                       fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules)
 {
        fr_sbuff_t      our_in = FR_SBUFF(in);
        xlat_exp_head_t *head;
@@ -1704,7 +1724,7 @@ fr_slen_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t *in,
        MEM(head = xlat_exp_head_alloc(ctx));
        fr_strerror_clear();    /* Clear error buffer */
 
-       if (xlat_tokenize_input(head, &our_in, p_rules, t_rules, literals_safe_for) < 0) {
+       if (xlat_tokenize_input(head, &our_in, p_rules, t_rules) < 0) {
                talloc_free(head);
                FR_SBUFF_ERROR_RETURN(&our_in);
        }
index bead038b6e4894949a7cfc719a589e9da6d6481c..9a4601ab53f90cce896ccff69b2b894c4ddef30c 100644 (file)
@@ -463,7 +463,7 @@ static int call_env_query_parse(TALLOC_CTX *ctx, void *out, tmpl_rules_t const *
                                        ['%'] = '%',
                                        ['\\'] = '\\',
                                },
-                 }}, t_rules, 0) < 0) {
+                 }}, t_rules) < 0) {
                talloc_free(query);
                return -1;
        }