From: Arran Cudbard-Bell Date: Fri, 3 Dec 2021 14:39:05 +0000 (-0600) Subject: Fix xlat escaping for legacy synchronous calls X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8d80eccfb6640c8740e417c389269fa694c9a928;p=thirdparty%2Ffreeradius-server.git Fix xlat escaping for legacy synchronous calls --- diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index dab8bc01bef..be24e382a39 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -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); diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index 49b2e5d14d7..ce9ac35f8d5 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -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. diff --git a/src/tests/keywords/regex-escape b/src/tests/keywords/regex-escape index 162a2e98bb8..9100edacf49 100644 --- a/src/tests/keywords/regex-escape +++ b/src/tests/keywords/regex-escape @@ -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}/) { diff --git a/src/tests/keywords/xlat-list b/src/tests/keywords/xlat-list index 84f549f0c50..1ef22084923 100644 --- a/src/tests/keywords/xlat-list +++ b/src/tests/keywords/xlat-list @@ -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 } diff --git a/src/tests/keywords/xlat-octets b/src/tests/keywords/xlat-octets index 55c082619a5..16b1527f36b 100644 --- a/src/tests/keywords/xlat-octets +++ b/src/tests/keywords/xlat-octets @@ -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 } diff --git a/src/tests/keywords/xlat-sub b/src/tests/keywords/xlat-sub index 5b8db068d04..15766cc4747 100644 --- a/src/tests/keywords/xlat-sub +++ b/src/tests/keywords/xlat-sub @@ -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 } } diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt index 35d2fe57fac..1bd86a4c37e 100644 --- a/src/tests/xlat/expr.txt +++ b/src/tests/xlat/expr.txt @@ -17,7 +17,7 @@ xlat %{expr: 6 + -(1 + 3)} data 2 xlat %{tolower:\%{ FOO} -data \%{ foo +data %{ foo xlat \%D data %D