From f20bc2ae70ede35800aede39bcace88aba1cdc19 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Wed, 1 Dec 2021 18:16:59 -0600 Subject: [PATCH] Only apply escape functions to tainted boxes --- src/lib/server/tmpl_eval.c | 8 ++-- src/lib/unlang/xlat_eval.c | 58 +++++++++++++++-------- src/tests/keywords/escape-sequences | 2 +- src/tests/keywords/redundant | 4 +- src/tests/keywords/redundant-load-balance | 4 +- src/tests/keywords/xlat-attr-index | 2 +- 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index 7130ccddde7..dab8bc01bef 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -324,7 +324,9 @@ ssize_t _tmpl_to_type(void *out, size_t len; RDEBUG4("EXPAND TMPL XLAT PARSED"); - RDEBUG2("EXPAND %s", vpt->name); /* xlat_struct doesn't do this */ + + /* No EXPAND here as the xlat code does it */ + if (!buff) { fr_strerror_const("Missing expansion buffer for XLAT_STRUCT"); return -1; @@ -638,7 +640,7 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out, char *result; RDEBUG4("EXPAND TMPL XLAT STRUCT"); - RDEBUG2("EXPAND %s", vpt->name); /* xlat_struct doesn't do this */ + /* No EXPAND xlat here as the xlat code does it */ /* Error in expansion, this is distinct from zero length expansion */ slen = xlat_aeval_compiled(tmp_ctx, &result, request, tmpl_xlat(vpt), escape, escape_ctx); @@ -657,8 +659,6 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out, fr_value_box_bstrndup_shallow(&value, NULL, tmp.vb_strvalue, tmp.vb_length, tmp.tainted); to_cast = &value; - - RDEBUG2(" --> %s", value.vb_strvalue); /* Print post-unescaping */ } break; diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index 243a721e905..49b2e5d14d7 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -1353,34 +1353,68 @@ static ssize_t xlat_eval_sync(TALLOC_CTX *ctx, char **out, request_t *request, x xlat_escape_legacy_t escape, void const *escape_ctx) { fr_value_box_list_t result; + bool success = false; TALLOC_CTX *pool = talloc_new(NULL); + rlm_rcode_t rcode; char *str; XLAT_DEBUG("xlat_eval_sync"); + *out = NULL; + fr_value_box_list_init(&result); /* * Use the unlang stack to evaluate * the async xlat up until the point * that it needs to yield. */ - if (unlang_xlat_push(pool, NULL, &result, request, head, true) < 0) { + if (unlang_xlat_push(pool, &success, &result, request, head, true) < 0) { talloc_free(pool); - return NULL; + return -1; } - switch (unlang_interpret_synchronous(unlang_interpret_event_list(request), request)) { + rcode = unlang_interpret_synchronous(unlang_interpret_event_list(request), request); + switch (rcode) { default: break; case RLM_MODULE_REJECT: case RLM_MODULE_FAIL: + eval_failed: RPEDEBUG("xlat evaluation failed"); talloc_free(pool); - return NULL; + return -1; } + if (!success) goto eval_failed; if (!fr_dlist_empty(&result)) { + if (escape) { + fr_value_box_t *vb = NULL; + + /* + * For tainted boxes perform the requested escaping + */ + while ((vb = fr_dlist_next(&result, vb))) { + fr_dlist_t entry; + size_t len, real_len; + char *escaped; + + if (!vb->tainted) continue; + + len = talloc_array_length(str) * 3; + + escaped = talloc_array(pool, char, len); + real_len = escape(request, escaped, len, str, UNCONST(void *, escape_ctx)); + + entry = vb->entry; + fr_value_box_clear_value(vb); + fr_value_box_bstrndup(vb, vb, NULL, escaped, real_len, false); + vb->entry = entry; + + talloc_free(escaped); + } + } + str = fr_value_box_list_aprint(ctx, &result, NULL, &fr_value_escape_double); if (!str) { RPEDEBUG("Failed concatenating xlat result string"); @@ -1392,18 +1426,6 @@ static ssize_t xlat_eval_sync(TALLOC_CTX *ctx, char **out, request_t *request, x } talloc_free(pool); /* Memory should be in new ctx */ - if (escape) { - size_t len; - char *escaped; - - len = talloc_array_length(str) * 3; - - escaped = talloc_array(ctx, char, len); - escape(request, escaped, len, str, UNCONST(void *, escape_ctx)); - talloc_free(str); - str = escaped; - } - *out = str; return strlen(str); @@ -1476,7 +1498,6 @@ ssize_t _xlat_eval(TALLOC_CTX *ctx, char **out, size_t outlen, request_t *reques ssize_t len; xlat_exp_t *node; - RDEBUG2("EXPAND %s", fmt); RINDENT(); /* @@ -1506,7 +1527,6 @@ ssize_t _xlat_eval(TALLOC_CTX *ctx, char **out, size_t outlen, request_t *reques talloc_free(node); REXDENT(); - RDEBUG2("--> %s", *out); return len; } @@ -1561,7 +1581,7 @@ ssize_t xlat_aeval_compiled(TALLOC_CTX *ctx, char **out, request_t *request, * - >0 on success which is argc to the corresponding argv */ int xlat_aeval_compiled_argv(TALLOC_CTX *ctx, char ***argv, request_t *request, - xlat_exp_t const *xlat, xlat_escape_legacy_t escape, void const *escape_ctx) + xlat_exp_t const *xlat, xlat_escape_legacy_t escape, void const *escape_ctx) { int i; ssize_t slen; diff --git a/src/tests/keywords/escape-sequences b/src/tests/keywords/escape-sequences index ffb8a0e7f4c..0c3310b94f2 100644 --- a/src/tests/keywords/escape-sequences +++ b/src/tests/keywords/escape-sequences @@ -54,7 +54,7 @@ if ("%{Tmp-String-0[1]}" != "0x01\0010x07\0070x0A\n0x0D\r\"\"0xb0\260°") { } # And another slightly different codepath... -if ("%{Tmp-String-0[*]}" != "i have scary embedded things\000 inside me,0x01\0010x07\0070x0A\n0x0D\r\"\"0xb0\260°") { +if ("%{Tmp-String-0[*]}" != "i have scary embedded things\000 inside me0x01\0010x07\0070x0A\n0x0D\r\"\"0xb0\260°") { test_fail } diff --git a/src/tests/keywords/redundant b/src/tests/keywords/redundant index f1c53cafe35..e338e729870 100644 --- a/src/tests/keywords/redundant +++ b/src/tests/keywords/redundant @@ -39,7 +39,7 @@ foreach &Tmp-Integer-1 { redundant { group { # fail on even numbered values, succeed on odd numbered ones - if ("%{expr:%{Foreach-Variable-0} %% 2}" == 0) { + if ("%{expr:%{Foreach-Variable-0} % 2}" == 0) { fail } else { @@ -52,7 +52,7 @@ foreach &Tmp-Integer-1 { } group { # succeed on even-numbered values, fail on off-numbered ones. - if ("%{expr:%{Foreach-Variable-0} %% 2}" == 1) { + if ("%{expr:%{Foreach-Variable-0} % 2}" == 1) { fail } else { diff --git a/src/tests/keywords/redundant-load-balance b/src/tests/keywords/redundant-load-balance index 60b5f520fbf..74c6e18552b 100644 --- a/src/tests/keywords/redundant-load-balance +++ b/src/tests/keywords/redundant-load-balance @@ -25,7 +25,7 @@ foreach &Tmp-Integer-1 { redundant-load-balance { group { # fail on even numbered values, succeed on odd numbered ones - if ("%{expr:%{Foreach-Variable-0} %% 2}" == 0) { + if ("%{expr:%{Foreach-Variable-0} % 2}" == 0) { fail } else { @@ -38,7 +38,7 @@ foreach &Tmp-Integer-1 { } group { # succeed on even-numbered values, fail on off-numbered ones. - if ("%{expr:%{Foreach-Variable-0} %% 2}" == 1) { + if ("%{expr:%{Foreach-Variable-0} % 2}" == 1) { fail } else { diff --git a/src/tests/keywords/xlat-attr-index b/src/tests/keywords/xlat-attr-index index 5d9757acb56..039f465663e 100644 --- a/src/tests/keywords/xlat-attr-index +++ b/src/tests/keywords/xlat-attr-index @@ -15,7 +15,7 @@ if (("%{Tmp-IP-Address-0[0]}" != 192.0.2.1) || ("%{Tmp-IP-Address-0[1]}" != 192. test_fail } -if ("%{Tmp-IP-Address-0[*]}" != '192.0.2.1,192.0.2.2') { +if ("%{Tmp-IP-Address-0[*]}" != '192.0.2.1192.0.2.2') { test_fail } -- 2.47.3