]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Fix xlat escaping for legacy synchronous calls
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Fri, 3 Dec 2021 14:39:05 +0000 (08:39 -0600)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sat, 4 Dec 2021 19:22:17 +0000 (14:22 -0500)
src/lib/server/tmpl_eval.c
src/lib/unlang/xlat_eval.c
src/tests/keywords/regex-escape
src/tests/keywords/xlat-list
src/tests/keywords/xlat-octets
src/tests/keywords/xlat-sub
src/tests/xlat/expr.txt

index dab8bc01bef45ac5f84a779099381bf4c1ac671e..be24e382a390d9eee1dd7d0ed02cce6dfeb6a5e5 100644 (file)
@@ -624,7 +624,7 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out,
                 */
                ret = fr_value_box_from_str(tmp_ctx, &tmp, src_type, NULL,
                                            result, (size_t)slen,
-                                           &fr_value_unescape_double, false);
+                                           NULL, false);
                if (ret < 0) goto error;
 
                fr_value_box_bstrndup_shallow(&value, NULL, tmp.vb_strvalue, tmp.vb_length, tmp.tainted);
@@ -654,7 +654,7 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out,
                 */
                ret = fr_value_box_from_str(tmp_ctx, &tmp, src_type, NULL,
                                            result, (size_t)slen,
-                                           &fr_value_unescape_double, false);
+                                           NULL, false);
                if (ret < 0) goto error;
 
                fr_value_box_bstrndup_shallow(&value, NULL, tmp.vb_strvalue, tmp.vb_length, tmp.tainted);
index 49b2e5d14d7b0d872d759e8f359f90fa9e211c0f..ce9ac35f8d59a72da2b8fd6747443c9a32972635 100644 (file)
@@ -470,7 +470,8 @@ xlat_action_t xlat_process_args(TALLOC_CTX *ctx, fr_value_box_list_t *list, requ
  *     - #XLAT_ACTION_DONE     if we're done processing this node.
  *
  */
-static xlat_action_t xlat_eval_one_letter(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, char letter)
+static inline CC_HINT(always_inline)
+xlat_action_t xlat_eval_one_letter(TALLOC_CTX *ctx, fr_value_box_list_t *out, request_t *request, char letter)
 {
 
        char            buffer[64];
@@ -636,8 +637,7 @@ static xlat_action_t xlat_eval_one_letter(TALLOC_CTX *ctx, fr_dcursor_t *out, re
                return XLAT_ACTION_FAIL;
        }
 
-       fr_dcursor_append(out, value);
-       fr_dcursor_next(out);                           /* Advance to our first value */
+       fr_dlist_insert_tail(out, value);
 
        return XLAT_ACTION_DONE;
 }
@@ -656,7 +656,8 @@ static xlat_action_t xlat_eval_one_letter(TALLOC_CTX *ctx, fr_dcursor_t *out, re
  *     - #XLAT_ACTION_FAIL     on memory allocation errors.
  *     - #XLAT_ACTION_DONE     if we're done processing this node.
  */
-static xlat_action_t xlat_eval_pair_virtual(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, tmpl_t const *vpt)
+static xlat_action_t xlat_eval_pair_virtual(TALLOC_CTX *ctx, fr_value_box_list_t *out,
+                                           request_t *request, tmpl_t const *vpt)
 {
        fr_radius_packet_t      *packet = NULL;
        fr_value_box_t  *value;
@@ -790,8 +791,7 @@ static xlat_action_t xlat_eval_pair_virtual(TALLOC_CTX *ctx, fr_dcursor_t *out,
        }
 
 done:
-       fr_dcursor_append(out, value);
-       fr_dcursor_next(out);                           /* Advance to our first value */
+       fr_dlist_insert_tail(out, value);
 
        return XLAT_ACTION_DONE;
 }
@@ -807,7 +807,8 @@ done:
  *     - #XLAT_ACTION_FAIL             we failed getting a value for the attribute.
  *     - #XLAT_ACTION_DONE             we successfully evaluated the xlat.
  */
-static xlat_action_t xlat_eval_pair_real(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, tmpl_t const *vpt)
+static inline CC_HINT(always_inline) xlat_action_t
+xlat_eval_pair_real(TALLOC_CTX *ctx, fr_value_box_list_t *out, request_t *request, tmpl_t const *vpt)
 {
        fr_pair_t               *vp = NULL;
        fr_value_box_t          *value;
@@ -849,8 +850,7 @@ static xlat_action_t xlat_eval_pair_real(TALLOC_CTX *ctx, fr_dcursor_t *out, req
                                goto done;
                        }
                        value->datum.int32 = 0;
-                       fr_dcursor_append(out, value);
-                       fr_dcursor_next(out);                   /* Advance to our first value */
+                       fr_dlist_insert_tail(out, value);
                } /* Fall through to being done */
 
                goto done;
@@ -871,8 +871,7 @@ static xlat_action_t xlat_eval_pair_real(TALLOC_CTX *ctx, fr_dcursor_t *out, req
 
                value = fr_value_box_alloc(ctx, FR_TYPE_UINT32, NULL, false);
                value->datum.uint32 = count;
-               fr_dcursor_append(out, value);
-               fr_dcursor_next(out);                           /* Advance to our first value */
+               fr_dlist_insert_tail(out, value);
                break;
        }
 
@@ -891,9 +890,8 @@ static xlat_action_t xlat_eval_pair_real(TALLOC_CTX *ctx, fr_dcursor_t *out, req
                     vp = fr_dcursor_next(&cursor)) {
                        value = fr_value_box_alloc(ctx, vp->data.type, vp->da, vp->data.tainted);
                        fr_value_box_copy(value, value, &vp->data);
-                       fr_dcursor_append(out, value);
+                       fr_dlist_insert_tail(out, value);
                }
-               fr_dcursor_next(out);                           /* Advance to our first value */
                break;
 
        default:
@@ -906,8 +904,7 @@ static xlat_action_t xlat_eval_pair_real(TALLOC_CTX *ctx, fr_dcursor_t *out, req
                if (!value) goto oom;
 
                fr_value_box_copy(value, value, &vp->data);     /* Also dups taint */
-               fr_dcursor_append(out, value);
-               fr_dcursor_next(out);                           /* Advance to our first value */
+               fr_dlist_insert_tail(out, value);
                break;
        }
 
@@ -1045,11 +1042,11 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                        fr_dlist_talloc_free(&result_copy);
                        return xa;
                }
-               VALUE_BOX_TALLOC_LIST_VERIFY(result);
 
+               VALUE_BOX_TALLOC_LIST_VERIFY(result);
                xa = node->call.func->func(ctx, out,
-                                                XLAT_CTX(node->call.inst->data, t->data, t->mctx, NULL),
-                                                request, result);
+                                          XLAT_CTX(node->call.inst->data, t->data, t->mctx, NULL),
+                                          request, result);
                VALUE_BOX_TALLOC_LIST_VERIFY(result);
 
                if (RDEBUG_ENABLED2) xlat_debug_log_expansion(request, *in, &result_copy);
@@ -1064,7 +1061,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                        return xa;
 
                case XLAT_ACTION_PUSH_UNLANG:
-                       RDEBUG3("  -- UNLANG");
+                       RDEBUG3("   -- UNLANG");
                        return xa;
 
                case XLAT_ACTION_YIELD:
@@ -1188,6 +1185,10 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
 {
        xlat_exp_t const        *node = *in;
        xlat_action_t           xa = XLAT_ACTION_DONE;
+       fr_value_box_list_t     result;         /* tmp list so debug works correctly */
+
+       fr_value_box_list_init(&result);
+
        fr_value_box_t          *value;
 
        *child = NULL;
@@ -1201,6 +1202,8 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
                fr_dcursor_tail(out);   /* Needed for debugging */
                VALUE_BOX_TALLOC_LIST_VERIFY(out->dlist);
 
+               fr_assert(fr_dlist_num_elements(&result) == 0); /* Should all have been moved */
+
                switch (node->type) {
                case XLAT_BOX:
                        XLAT_DEBUG("** [%i] %s(value_box) - %s", unlang_interpret_stack_depth(request), __FUNCTION__, node->fmt);
@@ -1228,13 +1231,14 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
                                   node->fmt);
 
                        xlat_debug_log_expansion(request, node, NULL);
-                       if (xlat_eval_one_letter(ctx, out, request, node->fmt[0]) == XLAT_ACTION_FAIL) {
+                       if (xlat_eval_one_letter(ctx, &result, request, node->fmt[0]) == XLAT_ACTION_FAIL) {
                        fail:
-                               fr_dcursor_free_list(out);      /* Only frees what we've added during this call */
+                               fr_dlist_talloc_free(&result);
                                xa = XLAT_ACTION_FAIL;
                                goto finish;
                        }
-                       xlat_debug_log_result(request, fr_dcursor_current(out));
+                       xlat_debug_log_list_result(request, &result);
+                       fr_dlist_move(out->dlist, &result);
                        continue;
 
                case XLAT_ATTRIBUTE:
@@ -1242,8 +1246,10 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
                                   node->fmt);
 
                        xlat_debug_log_expansion(request, node, NULL);
-                       if (xlat_eval_pair_real(ctx, out, request, node->attr) == XLAT_ACTION_FAIL) goto fail;
-                       xlat_debug_log_result(request, fr_dcursor_current(out));
+                       if (xlat_eval_pair_real(ctx, &result, request, node->attr) == XLAT_ACTION_FAIL) goto fail;
+
+                       xlat_debug_log_list_result(request, &result);
+                       fr_dlist_move(out->dlist, &result);
                        continue;
 
                case XLAT_VIRTUAL:
@@ -1262,10 +1268,6 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
                        continue;
 
                case XLAT_FUNC:
-               {
-                       fr_value_box_list_t result;
-                       fr_value_box_list_init(&result);
-
                        XLAT_DEBUG("** [%i] %s(func) - %%{%s:...}", unlang_interpret_stack_depth(request), __FUNCTION__,
                                   node->fmt);
 
@@ -1285,7 +1287,6 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
                         */
                        xa = xlat_frame_eval_repeat(ctx, out, child, NULL, request, in, &result);
                        if (xa != XLAT_ACTION_DONE || (!*in)) goto finish;
-               }
                        continue;
 
 #ifdef HAVE_REGEX
@@ -1295,12 +1296,16 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_t con
 
                        XLAT_DEBUG("** [%i] %s(regex) - %%{%s}", unlang_interpret_stack_depth(request), __FUNCTION__,
                                   node->fmt);
+
+                       xlat_debug_log_expansion(request, node, NULL);
                        MEM(value = fr_value_box_alloc_null(ctx));
                        if (regex_request_to_sub(ctx, &str, request, node->regex_index) < 0) {
                                talloc_free(value);
                                continue;
                        }
                        fr_value_box_bstrdup_buffer_shallow(NULL, value, NULL, str, false);
+
+                       xlat_debug_log_result(request, value);
                        fr_dcursor_append(out, value);
                }
                        continue;
@@ -1401,10 +1406,16 @@ static ssize_t xlat_eval_sync(TALLOC_CTX *ctx, char **out, request_t *request, x
 
                                if (!vb->tainted) continue;
 
-                               len = talloc_array_length(str) * 3;
+                               if (fr_value_box_cast_in_place(pool, vb, FR_TYPE_STRING, NULL) < 0) {
+                                       RPEDEBUG("Failed casting result to string");
+                               error:
+                                       talloc_free(pool);
+                                       return -1;
+                               }
 
+                               len = vb->vb_length * 3;
                                escaped = talloc_array(pool, char, len);
-                               real_len = escape(request, escaped, len, str, UNCONST(void *, escape_ctx));
+                               real_len = escape(request, escaped, len, vb->vb_strvalue, UNCONST(void *, escape_ctx));
 
                                entry = vb->entry;
                                fr_value_box_clear_value(vb);
@@ -1415,11 +1426,11 @@ static ssize_t xlat_eval_sync(TALLOC_CTX *ctx, char **out, request_t *request, x
                        }
                }
 
-               str = fr_value_box_list_aprint(ctx, &result, NULL, &fr_value_escape_double);
+               str = fr_value_box_list_aprint(ctx, &result, NULL, NULL);
                if (!str) {
                        RPEDEBUG("Failed concatenating xlat result string");
                        talloc_free(pool);
-                       return -1;
+                       goto error;
                }
        } else {
                str = talloc_strdup(ctx, "");
@@ -1428,7 +1439,7 @@ static ssize_t xlat_eval_sync(TALLOC_CTX *ctx, char **out, request_t *request, x
 
        *out = str;
 
-       return strlen(str);
+       return talloc_array_length(str) - 1;
 }
 
 /** Replace %whatever in a string.
@@ -1459,8 +1470,6 @@ static ssize_t _xlat_eval_compiled(TALLOC_CTX *ctx, char **out, size_t outlen, r
                return slen;
        }
 
-       slen = strlen(buff);
-
        /*
         *      If out doesn't point to an existing buffer
         *      copy the pointer to our buffer over.
index 162a2e98bb8bb481071497a6cd89247b753ff870..9100edacf4935175b028164fc2f7f33022a825df 100644 (file)
@@ -7,8 +7,8 @@
 #  characters escaped.  Because the input strings are unsafe.
 #
 update request {
-       &Tmp-String-0 := "example.com"
-       &Tmp-String-1 := "exampleXcom"
+       &Tmp-String-0 := "%{taint:example.com}"
+       &Tmp-String-1 := "%{taint:exampleXcom}"
 }
 
 if ("exampleXcom" =~ /%{Tmp-String-0}/) {
index 84f549f0c502cef0d616aef7fc34f8a5d2f19821..1ef22084923d91bbac421d10675a55ed179ef3fa 100644 (file)
@@ -24,7 +24,7 @@ if (("%{control[n]}" != 192.0.2.2)) {
        test_fail
 }
 
-if ("%{control[*]}" != '192.0.2.1,192.0.2.2') {
+if ("%{control[*]}" != '192.0.2.1192.0.2.2') {
        test_fail
 }
 
index 55c082619a5409e0c08377fb881020628542ffbf..16b1527f36bef5aaaedecb2b664b40546606182a 100644 (file)
@@ -18,7 +18,7 @@ if ("%{Tmp-Octets-0[0]}" != '0x0001020304050607') {
        test_fail
 }
 
-if ("%{Tmp-Octets-0[*]}" != '0x0001020304050607,0x0706050403020100') {
+if ("%{Tmp-Octets-0[*]}" != '0x00010203040506070x0706050403020100') {
        test_fail
 }
 
index 5b8db068d04b1483b9ed3ab87d77145c440c780a..15766cc474767449fde3f621f58380f0dbb97da8 100644 (file)
@@ -110,16 +110,20 @@ if ("%(sub:%{Tmp-String-0} /***/g .)" != '') {
        test_fail
 }
 
-if (&Module-Failure-Message[1] != 'Failed compiling regex: quantifier does not follow a repeatable item') {
+if (&Module-Failure-Message[3] != 'Failed compiling regex: quantifier does not follow a repeatable item') {
        test_fail
 }
 
+update request {
+       &Module-Failure-Message !* ANY
+}
+
 # Empty regex
 if ("%(sub:%{Tmp-String-0} //g .)" != '') {
        test_fail
 }
 
-if (&Module-Failure-Message[1] != 'Failed compiling regex: Empty expression') {
+if (&Module-Failure-Message[3] != 'Failed compiling regex: Empty expression') {
        test_fail
 }
 }
index 35d2fe57fac093d3228b4c569b62980e408914fb..1bd86a4c37e69d0e7db46f95074a92c174b7cf68 100644 (file)
@@ -17,7 +17,7 @@ xlat %{expr: 6 + -(1 + 3)}
 data 2
 
 xlat %{tolower:\%{ FOO}
-data \%{ foo
+data %{ foo
 
 xlat \%D
 data %D