From: Alan T. DeKok Date: Sat, 29 Mar 2025 23:20:16 +0000 (-0400) Subject: track safety of regex data X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b5c017bacc153369d155ec2012fb7d15eba1974d;p=thirdparty%2Ffreeradius-server.git track safety of regex data so that if we do regex captures of something which is SAFE_FOR_SQL, the resulting string is also SAFE_FOR_SQL. There are odd cases where this _might_ be wrong, such as when the capture text crosses a quoted string boundary. But that is arguably the fault of the admin who created the offending regex, and decided to use it in an unsafe manner. --- diff --git a/src/lib/server/paircmp.c b/src/lib/server/paircmp.c index d133e58a139..3f74a6f3ef5 100644 --- a/src/lib/server/paircmp.c +++ b/src/lib/server/paircmp.c @@ -132,7 +132,7 @@ int paircmp_pairs(UNUSED request_t *request, fr_pair_t const *check, fr_pair_t * /* * Add in %{0}. %{1}, etc. */ - regex_sub_to_request(request, &preg, ®match); + regex_sub_to_request(request, &preg, ®match, &vp->data); ret = (slen == 1) ? 0 : -1; } else { ret = (slen != 1) ? 0 : -1; diff --git a/src/lib/server/regex.c b/src/lib/server/regex.c index 1fea13a1577..139649ce6fd 100644 --- a/src/lib/server/regex.c +++ b/src/lib/server/regex.c @@ -38,6 +38,9 @@ typedef struct { regex_t *preg; //!< Compiled pattern. #endif fr_regmatch_t *regmatch; //!< Match vectors. + + fr_value_box_safe_for_t safe_for; + bool secret; } fr_regcapture_t; /** Adds subcapture values to request data @@ -53,8 +56,9 @@ typedef struct { * reparented to the regcapture struct. * @param[in,out] regmatch Pointers into value. May be set to NULL if * reparented to the regcapture struct. + * @param[in] in value-box which we matched against */ -void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **regmatch) +void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **regmatch, fr_value_box_t const *in) { fr_regcapture_t *old_rc, *new_rc; /* lldb doesn't like bare new *sigh* */ @@ -73,6 +77,7 @@ void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **re fr_assert(preg && *preg); fr_assert(regmatch); + fr_assert(in); DEBUG4("Adding %zu matches", (*regmatch)->used); @@ -97,6 +102,8 @@ void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **re * Steal match data */ new_rc->regmatch = talloc_steal(new_rc, *regmatch); + new_rc->safe_for = in->safe_for; + new_rc->secret = in->secret; *regmatch = NULL; request_data_talloc_add(request, request, REQUEST_DATA_REGEX, fr_regcapture_t, new_rc, true, false, false); @@ -115,7 +122,7 @@ void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **re * - 0 on success. * - -1 on notfound. */ -int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32_t num) +int regex_request_to_sub(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, uint32_t num) { fr_regcapture_t *rc; char *buff; @@ -126,7 +133,6 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 rc = request_data_reference(request, request, REQUEST_DATA_REGEX); if (!rc) { RDEBUG4("No subcapture data found"); - *out = NULL; return -1; } match_data = talloc_get_type_abort(rc->regmatch->match_data, pcre2_match_data); @@ -142,22 +148,22 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 */ case PCRE2_ERROR_NOSUBSTRING: RDEBUG4("%i/%zu Not found", num + 1, rc->regmatch->used); - *out = NULL; return -1; default: if (ret < 0) { - *out = NULL; return -1; } MEM(buff = talloc_array(ctx, char, ++len)); /* +1 for \0, it'll get reset by pcre2_substring */ pcre2_substring_copy_bynumber(match_data, num, (PCRE2_UCHAR *)buff, &len); /* can't error */ - RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, - fr_box_strvalue_buffer(buff), talloc_array_length(buff) - 1); + fr_value_box_init(out, FR_TYPE_STRING, NULL, false); + fr_value_box_bstrndup_shallow(out, NULL, buff, len, false); + fr_value_box_mark_safe_for(out, rc->safe_for); + fr_value_box_set_secret(out, rc->secret); - *out = buff; + RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, out, out->vb_length); break; } @@ -176,7 +182,7 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 * - 0 on success. * - -1 on notfound. */ -int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, char const *name) +int regex_request_to_sub_named(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, char const *name) { fr_regcapture_t *rc; char *buff; @@ -187,7 +193,6 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, rc = request_data_reference(request, request, REQUEST_DATA_REGEX); if (!rc) { RDEBUG4("No subcapture data found"); - *out = NULL; return -1; } match_data = rc->regmatch->match_data; @@ -203,22 +208,20 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, */ case PCRE2_ERROR_NOSUBSTRING: RDEBUG4("No named capture group \"%s\"", name); - *out = NULL; return -1; default: - if (ret < 0) { - *out = NULL; - return -1; - } + if (ret < 0) return -1; MEM(buff = talloc_array(ctx, char, ++len)); /* +1 for \0, it'll get reset by pcre2_substring */ pcre2_substring_copy_byname(match_data, (PCRE2_UCHAR const *)name, (PCRE2_UCHAR *)buff, &len); /* can't error */ - RDEBUG4("Found \"%s\": %pV (%zu)", name, - fr_box_strvalue_buffer(buff), talloc_array_length(buff) - 1); + fr_value_box_init(out, FR_TYPE_STRING, NULL, false); + fr_value_box_bstrndup_shallow(out, NULL, buff, len, false); + fr_value_box_mark_safe_for(out, rc->safe_for); + fr_value_box_set_secret(out, rc->secret); - *out = buff; + RDEBUG4("Found \"%s\": %pV (%zu)", name, out, out->vb_length); break; } @@ -237,7 +240,7 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, * - 0 on success. * - -1 on notfound. */ -int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32_t num) +int regex_request_to_sub(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, uint32_t num) { fr_regcapture_t *rc; char const *p; @@ -246,7 +249,6 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 rc = request_data_reference(request, request, REQUEST_DATA_REGEX); if (!rc) { RDEBUG4("No subcapture data found"); - *out = NULL; return -1; } @@ -262,22 +264,20 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 */ case PCRE_ERROR_NOSUBSTRING: RDEBUG4("%i/%zu Not found", num + 1, rc->regmatch->used); - *out = NULL; return -1; default: - if (ret < 0) { - *out = NULL; - return -1; - } + if (ret < 0) return -1; talloc_set_type(p, char); - p = talloc_steal(ctx, p); + talloc_steal(ctx, p); - RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, - fr_box_strvalue_buffer(p), talloc_array_length(p) - 1); + fr_value_box_init(out, FR_TYPE_STRING, NULL, false); + fr_value_box_bstrndup_shallow(out, NULL, p, talloc_array_length(p) - 1, false); + fr_value_box_mark_safe_for(out, rc->safe_for); + fr_value_box_set_secret(out, rc->secret); - memcpy(out, &p, sizeof(*out)); + RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, out, out->vb_length); break; } @@ -296,7 +296,7 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 * - 0 on success. * - -1 on notfound. */ -int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, char const *name) +int regex_request_to_sub_named(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, char const *name) { fr_regcapture_t *rc; void *rd; @@ -306,7 +306,6 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, rd = request_data_reference(request, request, REQUEST_DATA_REGEX); if (!rd) { RDEBUG4("No subcapture data found"); - *out = NULL; return -1; } @@ -323,26 +322,20 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, */ case PCRE_ERROR_NOSUBSTRING: RDEBUG4("No named capture group \"%s\"", name); - *out = NULL; return -1; default: - if (ret < 0) { - *out = NULL; - return -1; - } + if (ret < 0) return -1; - /* - * Check libpcre really is using our overloaded - * memory allocation and freeing talloc wrappers. - */ talloc_set_type(p, char); talloc_steal(ctx, p); - RDEBUG4("Found \"%s\": %pV (%zu)", name, - fr_box_strvalue_buffer(p), talloc_array_length(p) - 1); + fr_value_box_init(out, FR_TYPE_STRING, NULL, false); + fr_value_box_bstrndup_shallow(out, NULL, p, talloc_array_length(p) - 1, false); + fr_value_box_mark_safe_for(out, rc->safe_for); + fr_value_box_set_secret(out, rc->secret); - memcpy(out, &p, sizeof(*out)); + RDEBUG4("Found \"%s\": %pV (%zu)", name, out, out->vb_length); break; } @@ -362,7 +355,7 @@ int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, * - 0 on success. * - -1 on notfound. */ -int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32_t num) +int regex_request_to_sub(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, uint32_t num) { fr_regcapture_t *rc; char *buff; @@ -373,7 +366,6 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 rc = request_data_reference(request, request, REQUEST_DATA_REGEX); if (!rc) { RDEBUG4("No subcapture data found"); - *out = NULL; return -1; } match_data = rc->regmatch->match_data; @@ -385,7 +377,6 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 */ if ((num >= rc->regmatch->used) || (match_data[num].rm_eo == -1) || (match_data[num].rm_so == -1)) { RDEBUG4("%i/%zu Not found", num + 1, rc->regmatch->used); - *out = NULL; return -1; } @@ -399,9 +390,13 @@ int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32 len = match_data[num].rm_eo - match_data[num].rm_so; MEM(buff = talloc_bstrndup(ctx, start, len)); - RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, fr_box_strvalue_buffer(buff), len); - *out = buff; + fr_value_box_init(out, FR_TYPE_STRING, NULL, false); + fr_value_box_bstrndup_shallow(out, NULL, buff, len, false); + fr_value_box_mark_safe_for(out, rc->safe_for); + fr_value_box_set_secret(out, rc->secret); + + RDEBUG4("%i/%zu Found: %pV (%zu)", num + 1, rc->regmatch->used, out, out->vb_length); return 0; } diff --git a/src/lib/server/regex.h b/src/lib/server/regex.h index 0aa299c754a..3a79ae00b96 100644 --- a/src/lib/server/regex.h +++ b/src/lib/server/regex.h @@ -40,15 +40,15 @@ extern "C" { */ # define REQUEST_MAX_REGEX 32 -void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **regmatch); +void regex_sub_to_request(request_t *request, regex_t **preg, fr_regmatch_t **regmatch, fr_value_box_t const *in) CC_HINT(nonnull(1)); -int regex_request_to_sub(TALLOC_CTX *ctx, char **out, request_t *request, uint32_t num); +int regex_request_to_sub(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, uint32_t num) CC_HINT(nonnull); /* * Named capture groups only supported by PCRE. */ # if defined(HAVE_REGEX_PCRE2) || defined(HAVE_REGEX_PCRE) -int regex_request_to_sub_named(TALLOC_CTX *ctx, char **out, request_t *request, char const *name); +int regex_request_to_sub_named(TALLOC_CTX *ctx, fr_value_box_t *out, request_t *request, char const *name) CC_HINT(nonnull); # endif #endif diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index 2f2a134a9e6..8b4c0a82d36 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -2805,17 +2805,14 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_dcursor_t *out, */ if (!arg) { fr_value_box_t *vb; - char *p; MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(vb, &p, request, 0) < 0) { + if (regex_request_to_sub(vb, vb, request, 0) < 0) { REDEBUG2("No previous regex capture"); talloc_free(vb); return XLAT_ACTION_FAIL; } - fr_assert(p); - fr_value_box_bstrdup_buffer_shallow(NULL, vb, NULL, p, false); fr_dcursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2830,7 +2827,6 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_dcursor_t *out, { fr_value_box_t idx; fr_value_box_t *vb; - char *p; if (fr_value_box_list_next(in, in_head)) { REDEBUG("Only one subcapture argument allowed"); @@ -2843,13 +2839,11 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_dcursor_t *out, } MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(vb, &p, request, idx.vb_uint32) < 0) { + if (regex_request_to_sub(vb, vb, request, idx.vb_uint32) < 0) { REDEBUG2("No previous numbered regex capture group"); talloc_free(vb); return XLAT_ACTION_FAIL; } - fr_assert(p); - fr_value_box_bstrdup_buffer_shallow(NULL, vb, NULL, p, false); fr_dcursor_append(out, vb); return XLAT_ACTION_DONE; @@ -2858,7 +2852,6 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_dcursor_t *out, default: { fr_value_box_t *vb; - char *p; /* * Concatenate all input @@ -2872,14 +2865,11 @@ static xlat_action_t xlat_func_regex(TALLOC_CTX *ctx, fr_dcursor_t *out, } MEM(vb = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub_named(vb, &p, request, arg->vb_strvalue) < 0) { + if (regex_request_to_sub_named(vb, vb, request, arg->vb_strvalue) < 0) { REDEBUG2("No previous named regex capture group"); talloc_free(vb); return XLAT_ACTION_FAIL; } - - fr_assert(p); - fr_value_box_bstrdup_buffer_shallow(NULL, vb, NULL, p, false); fr_dcursor_append(out, vb); return XLAT_ACTION_DONE; diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index ee8eeb21114..efada4f30dc 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -1261,23 +1261,18 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_head_ #ifdef HAVE_REGEX case XLAT_REGEX: - { - char *str = NULL; - XLAT_DEBUG("** [%i] %s(regex) - %%{%s}", unlang_interpret_stack_depth(request), __FUNCTION__, node->fmt); xlat_debug_log_expansion(request, node, NULL, __LINE__); MEM(value = fr_value_box_alloc_null(ctx)); - if (regex_request_to_sub(ctx, &str, request, node->regex_index) < 0) { + if (regex_request_to_sub(value, value, 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, node, value); fr_dcursor_append(out, value); - } continue; #endif diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index cef7ca1c079..72c0339625d 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -596,6 +596,7 @@ static xlat_action_t xlat_regex_match(TALLOC_CTX *ctx, request_t *request, fr_va fr_sbuff_t *agg; char const *subject; size_t len; + fr_value_box_t safety = {}; FR_SBUFF_TALLOC_THREAD_LOCAL(&agg, 256, 8192); @@ -611,18 +612,21 @@ static xlat_action_t xlat_regex_match(TALLOC_CTX *ctx, request_t *request, fr_va if (vb->type == FR_TYPE_STRING) { subject = vb->vb_strvalue; len = vb->vb_length; + fr_value_box_safety_copy(&safety, vb); } else { fr_value_box_list_t list; fr_value_box_list_init(&list); fr_value_box_list_insert_head(&list, vb); + fr_value_box_mark_safe_for(&safety, FR_VALUE_BOX_SAFE_FOR_ANY); + vb = NULL; /* * Concatenate everything, and escape untrusted inputs. */ - if (fr_value_box_list_concat_as_string(NULL, agg, &list, NULL, 0, ®ex_escape_rules, + if (fr_value_box_list_concat_as_string(&safety, agg, &list, NULL, 0, ®ex_escape_rules, FR_VALUE_BOX_LIST_FREE_BOX, FR_REGEX_SAFE_FOR, true) < 0) { RPEDEBUG("Failed concatenating regular expression string"); talloc_free(regmatch); @@ -645,11 +649,11 @@ static xlat_action_t xlat_regex_match(TALLOC_CTX *ctx, request_t *request, fr_va return XLAT_ACTION_FAIL; case 0: - regex_sub_to_request(request, NULL, NULL); /* clear out old entries */ + regex_sub_to_request(request, NULL, NULL, NULL); /* clear out old entries */ continue; case 1: - regex_sub_to_request(request, preg, ®match); + regex_sub_to_request(request, preg, ®match, &safety); talloc_free(vb); goto done;