]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Make fr_uri_escape work as a value box escape function
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sat, 20 Jan 2024 02:19:09 +0000 (20:19 -0600)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sat, 20 Jan 2024 02:30:25 +0000 (20:30 -0600)
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.

src/lib/util/uri.c
src/lib/util/uri.h
src/modules/rlm_ldap/rlm_ldap.c
src/modules/rlm_rest/rlm_rest.c

index de220408f954f347f5a6723e14f2e330f16ae802..50c57c4067ef8b5c99d8f28d6549789e83b12107 100644 (file)
@@ -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;
index e634d87cb71dd36d1f5cd28d19c714d816a6ebc1..14ce1b8fcf6810ff812f9ebc8bf0d084dd845071 100644 (file)
@@ -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
 }
index 1afbe48c48e2365f7a5a31a5a1d32f21fe94d250..513b0ffd613e38cef65b695c06421b77c5583e33 100644 (file)
@@ -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;
        }
index 043498abcbe7c803e3e3cfad814c816d2f0f09fc..8fd9ce032ffab27c4b01028d8100ca192922ba0f 100644 (file)
@@ -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: