From: Arran Cudbard-Bell Date: Sat, 20 Jan 2024 02:19:09 +0000 (-0600) Subject: Make fr_uri_escape work as a value box escape function X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=622cbe312ea3e83bc6a4d1c1eff2514430951c33;p=thirdparty%2Ffreeradius-server.git Make fr_uri_escape work as a value box escape function No major changes here, we just record the current uri_part in a new fr_uri_escape_ctx_t struct. The original function is retained as fr_uri_escape_list which processes a list of value boxes. --- diff --git a/src/lib/util/uri.c b/src/lib/util/uri.c index de220408f95..50c57c4067e 100644 --- a/src/lib/util/uri.c +++ b/src/lib/util/uri.c @@ -24,100 +24,122 @@ RCSID("$Id$") #include "uri.h" -/** Parse a list of value boxes representing a URI +/** Escapes an individual value box that's part of a URI, advancing the pointer to uri_parts * - * Reads a URI from a list of value boxes and parses it according to the - * definition in uri_parts. Tainted values, where allowed, are escaped - * using the function specified for the uri part. + * @note This function has a signature compatible with fr_uri_escape_func_t. * - * @param uri to parse. A list of string type value boxes containing - * fragments of a URI. - * @param uri_parts definition of URI structure - * @param uctx to pass to escaping function + * @param[in,out] uri_vb to escape + * @param[in] uctx A fr_uri_escape_ctx_t containing the initial fr_uri_part_t + * and the uctx to pass to the escaping function. * @return - * - 0 on success - * - -1 on failure + * - 0 on success. + * - -1 on failure. */ -int fr_uri_escape(fr_value_box_list_t *uri, fr_uri_part_t const *uri_parts, void *uctx) +int fr_uri_escape(fr_value_box_t *uri_vb, void *uctx) { - fr_uri_part_t const *uri_part; + fr_uri_escape_ctx_t *ctx = uctx; fr_sbuff_t sbuff; uint8_t adv; - uri_part = uri_parts; - - fr_strerror_clear(); - - fr_value_box_list_foreach_safe(uri, uri_vb) { - if (uri_vb->tainted && !uri_part->tainted_allowed) { - fr_strerror_printf_push("Tainted value not allowed for %s", uri_part->name); + if (uri_vb->tainted && !ctx->uri_part->tainted_allowed) { + fr_strerror_printf_push("Tainted value not allowed for %s", ctx->uri_part->name); + return -1; + } + + /* + * Ensure boxes are strings before attempting to escape. + */ + if (unlikely(uri_vb->type != FR_TYPE_STRING)) { + if (unlikely(fr_value_box_cast_in_place(uri_vb, uri_vb, FR_TYPE_STRING, uri_vb->enumv) < 0)) { + fr_strerror_printf_push("Unable to cast %pV to a string", uri_vb); return -1; } + } - /* - * Ensure boxes are strings before attempting to escape. - */ - if (unlikely(uri_vb->type != FR_TYPE_STRING)) { - if (fr_value_box_cast_in_place(uri_vb, uri_vb, FR_TYPE_STRING, uri_vb->enumv) < 0) { - fr_strerror_printf_push("Unable to cast %pV to a string", uri_vb); - } - } - - /* - * Tainted boxes can only belong to a single part of the URI - */ - if (uri_vb->tainted) { - if (uri_part->func) { - /* - * Escaping often ends up breaking the vb's list pointers - * so remove it from the list and re-insert after the escaping - * has been done - */ - fr_value_box_t *prev = fr_value_box_list_remove(uri, uri_vb); - if (uri_part->func(uri_vb, uctx) < 0) { - fr_strerror_printf_push("Unable to escape tainted input %pV", uri_vb); - return -1; - } - fr_value_box_list_insert_after(uri, prev, uri_vb); + /* + * Tainted boxes can only belong to a single part of the URI + */ + if (uri_vb->tainted) { + if (ctx->uri_part->func) { + /* + * Escaping often ends up breaking the vb's list pointers + * so remove it from the list and re-insert after the escaping + * has been done + */ + fr_value_box_entry_t entry = uri_vb->entry; + if (ctx->uri_part->func(uri_vb, ctx->uctx) < 0) { + fr_strerror_printf_push("Unable to escape tainted input %pV", uri_vb); + return -1; } - continue; + uri_vb->entry = entry; } + return 0; + } + + /* + * This URI part has no term chars - so no need to look for them + */ + if (!ctx->uri_part->terminals) return 0; + + /* + * Zero length box - no terminators here + */ + if (uri_vb->vb_length == 0) return 0; + + /* + * Look for URI part terminator + */ + fr_sbuff_init_in(&sbuff, uri_vb->vb_strvalue, uri_vb->vb_length); + do { + fr_sbuff_adv_until(&sbuff, SIZE_MAX, ctx->uri_part->terminals, '\0'); /* - * This URI part has no term chars - so no need to look for them + * We've not found a terminal in the current box */ - if (!uri_part->terminals) continue; + adv = ctx->uri_part->part_adv[fr_sbuff_char(&sbuff, '\0')]; + if (adv == 0) continue; /* - * Zero length box - no terminators here + * This terminator has trailing characters to skip */ - if (uri_vb->vb_length == 0) continue; + if (ctx->uri_part->extra_skip) fr_sbuff_advance(&sbuff, ctx->uri_part->extra_skip); /* - * Look for URI part terminator + * Move to the next part */ - fr_sbuff_init_in(&sbuff, uri_vb->vb_strvalue, uri_vb->vb_length); + ctx->uri_part += adv; + if (!ctx->uri_part->terminals) break; + } while (fr_sbuff_advance(&sbuff, 1) > 0); - do { - fr_sbuff_adv_until(&sbuff, SIZE_MAX, uri_part->terminals, '\0'); + return 0; +} - /* - * We've not found a terminal in the current box - */ - adv = uri_part->part_adv[fr_sbuff_char(&sbuff, '\0')]; - if (adv == 0) continue; +/** Parse a list of value boxes representing a URI + * + * Reads a URI from a list of value boxes and parses it according to the + * definition in uri_parts. Tainted values, where allowed, are escaped + * using the function specified for the uri part. + * + * @param uri to parse. A list of string type value boxes containing + * fragments of a URI. + * @param uri_parts definition of URI structure. Should point to the start + * of the array of uri parts. + * @param uctx to pass to escaping function + * @return + * - 0 on success + * - -1 on failure + */ +int fr_uri_escape_list(fr_value_box_list_t *uri, fr_uri_part_t const *uri_parts, void *uctx) +{ + fr_uri_escape_ctx_t ctx = { + .uri_part = uri_parts, + .uctx = uctx, + }; - /* - * This terminator has trailing characters to skip - */ - if (uri_part->extra_skip) fr_sbuff_advance(&sbuff, uri_part->extra_skip); + fr_strerror_clear(); - /* - * Move to the next part - */ - uri_part += adv; - if (!uri_part->terminals) break; - } while (fr_sbuff_advance(&sbuff, 1) > 0); + fr_value_box_list_foreach_safe(uri, uri_vb) { + if (unlikely(fr_uri_escape(uri_vb, &ctx)) < 0) return -1; }} return 0; diff --git a/src/lib/util/uri.h b/src/lib/util/uri.h index e634d87cb71..14ce1b8fcf6 100644 --- a/src/lib/util/uri.h +++ b/src/lib/util/uri.h @@ -53,9 +53,21 @@ typedef struct { fr_uri_escape_func_t func; //!< Function to use to escape tainted values } fr_uri_part_t; +/** uctx to pass to fr_uri_escape + * + * @note Should not be passed to fr_uri_escape_list. That takes the uctx to pass to the fr_uri_escape_func_t directly. + */ +typedef struct { + fr_uri_part_t const *uri_part; //!< Start of the uri parts array. Will be updated + ///< as boxes are escaped. + void *uctx; //!< to pass to fr_uri_escape_func_t. +} fr_uri_escape_ctx_t; + #define XLAT_URI_PART_TERMINATOR { .name = NULL, .terminals = NULL, .tainted_allowed = false, .func = NULL } -int fr_uri_escape(fr_value_box_list_t *uri, fr_uri_part_t const *uri_parts, void *uctx); +int fr_uri_escape_list(fr_value_box_list_t *uri, fr_uri_part_t const *uri_parts, void *uctx); + +int fr_uri_escape(fr_value_box_t *uri_vb, void *uctx); #ifdef __cplusplus } diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c index 1afbe48c48e..513b0ffd613 100644 --- a/src/modules/rlm_ldap/rlm_ldap.c +++ b/src/modules/rlm_ldap/rlm_ldap.c @@ -629,7 +629,7 @@ static xlat_action_t ldap_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, XLAT_ARGS(in, &uri_components); - if (fr_uri_escape(&uri_components->vb_group, ldap_uri_parts, NULL) < 0){ + if (fr_uri_escape_list(&uri_components->vb_group, ldap_uri_parts, NULL) < 0){ RPERROR("Failed to escape LDAP URI"); return XLAT_ACTION_FAIL; } @@ -985,7 +985,7 @@ static xlat_action_t ldap_profile_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor XLAT_ARGS(in, &uri_components); - if (fr_uri_escape(&uri_components->vb_group, ldap_uri_parts, NULL) < 0) { + if (fr_uri_escape_list(&uri_components->vb_group, ldap_uri_parts, NULL) < 0) { RPERROR("Failed to escape LDAP URI"); return XLAT_ACTION_FAIL; } @@ -1230,7 +1230,7 @@ static unlang_action_t mod_map_proc(rlm_rcode_t *p_result, void *mod_inst, UNUSE ldap_map_ctx_t *map_ctx; char *host_url, *host = NULL; - if (fr_uri_escape(url, ldap_uri_parts, NULL) < 0) { + if (fr_uri_escape_list(url, ldap_uri_parts, NULL) < 0) { RPERROR("Failed to escape LDAP URI"); RETURN_MODULE_FAIL; } diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c index 043498abcbe..8fd9ce032ff 100644 --- a/src/modules/rlm_rest/rlm_rest.c +++ b/src/modules/rlm_rest/rlm_rest.c @@ -476,7 +476,7 @@ static xlat_action_t rest_xlat(UNUSED TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, randle->request = request; /* Populate the request pointer for escape callbacks */ - if (fr_uri_escape(&in_vb->vb_group, rest_uri_parts, randle) < 0) { + if (fr_uri_escape_list(&in_vb->vb_group, rest_uri_parts, randle) < 0) { RPEDEBUG("Failed escaping URI"); error: