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 d133e58a1390dc449109aa2a2c2c13cdbb3f25b4..3f74a6f3ef51f2ec95722ac540c3014c9b52fe44 100644 (file)
--- 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, &regmatch);
+                       regex_sub_to_request(request, &preg, &regmatch, &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 1fea13a1577208380035032ea8f74338ab376ca3..139649ce6fd5eb75ad1d1c6c2c50105ec8fefff8 100644 (file)
--- 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 0aa299c754a26d2b9406f9fa5ed1afa2aafee5cf..3a79ae00b96ff7343e423b2e9ec4316ff02e4b62 100644 (file)
--- 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 2f2a134a9e6cd312197f2d93270262aab4a17d8a..8b4c0a82d36edfe69cfb814057f843f72f6077f9 100644 (file)
--- 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 ee8eeb21114e44aa2b7611c34898e0430a635d3e..efada4f30dc28260497cddc570a9d457e2a6303c 100644 (file)
--- 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 cef7ca1c0795a833830b2f827ab1f50af9e26bbb..72c0339625da403c8b5a33b37dfbb3f7acec5569 100644 (file)
--- 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, &regex_escape_rules,
+                       if (fr_value_box_list_concat_as_string(&safety, agg, &list, NULL, 0, &regex_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, &regmatch);
+                       regex_sub_to_request(request, preg, &regmatch, &safety);
                        talloc_free(vb);
                        goto done;