Skip to content

Commit

Permalink
Remove taint support from regexes, and add regex.safe()
Browse files Browse the repository at this point in the history
  • Loading branch information
arr2036 committed Nov 9, 2024
1 parent b9d479a commit bc1b922
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 14 deletions.
33 changes: 33 additions & 0 deletions doc/antora/modules/reference/pages/unlang/condition/regex.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,39 @@ if (&User-Name =~ /^(.*)@example\.com$/) {
----
====

== Dynamic expressions and escaping

If the pattern contains a xref:xlat/index.adoc[dynamic expansion], elements
of the expansion could potentially affect pattern evaluation in unanticipated ways.

To prevent this, the server will, by default, escape any special characters
in the values produced by the dynamic expansion, so that they are treated as
literal characters in the regular expression.

To allow non-literals in dynamic elements of regular expressions, wrap any literal
values or expansions in `%regex.safe(...)`.

.Using regex.safe to prevent escaping
====
[source,unlang]
----
local pattern
switch (realm) {
case example.com {
pattern = %regex.safe('.*\.example\.com')
}
case test.com {
# '.' does not need to be escaped
pattern = 'test.com'
}
}
if (&Stripped-User-Name =~ /^%{pattern}$/) {
...
}
----
====

== Pre-Compiled vs Runtime Compiled

When the server starts any regular expressions comparisons it finds will be
Expand Down
2 changes: 1 addition & 1 deletion src/lib/server/tmpl_tokenize.c
Original file line number Diff line number Diff line change
Expand Up @@ -3485,7 +3485,7 @@ fr_slen_t tmpl_afrom_substr(TALLOC_CTX *ctx, tmpl_t **out,

vpt = tmpl_alloc_null(ctx);

slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, t_rules->literals_safe_for);
slen = xlat_tokenize(vpt, &head, &our_in, p_rules, t_rules, FR_REGEX_SAFE_FOR);
if (slen < 0) FR_SBUFF_ERROR_RETURN(&our_in);

/*
Expand Down
14 changes: 14 additions & 0 deletions src/lib/unlang/xlat_builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -4158,6 +4158,20 @@ do { \
if (unlikely((xlat = xlat_func_register(xlat_ctx, "regex", xlat_func_regex, FR_TYPE_STRING)) == NULL)) return -1;
xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
#endif

{
static xlat_arg_parser_t const xlat_regex_safe_args[] = {
{ .type = FR_TYPE_STRING, .variadic = true, .concat = true },
XLAT_ARG_PARSER_TERMINATOR
};

if (unlikely((xlat = xlat_func_register(xlat_ctx, "regex.safe",
xlat_transparent, FR_TYPE_STRING)) == NULL)) return -1;
xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL);
xlat_func_args_set(xlat, xlat_regex_safe_args);
xlat_func_safe_for_set(xlat, FR_REGEX_SAFE_FOR);
}

XLAT_REGISTER_PURE("sha1", xlat_func_sha1, FR_TYPE_OCTETS, xlat_func_sha_arg);

#ifdef HAVE_OPENSSL_EVP_H
Expand Down
4 changes: 2 additions & 2 deletions src/lib/unlang/xlat_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ static xlat_action_t xlat_regex_match(TALLOC_CTX *ctx, request_t *request, fr_va
* Concatenate everything, and escape untrusted inputs.
*/
if (fr_value_box_list_concat_as_string(NULL, NULL, agg, &list, NULL, 0, &regex_escape_rules,
FR_VALUE_BOX_LIST_FREE_BOX, true) < 0) {
FR_VALUE_BOX_LIST_FREE_BOX, FR_REGEX_SAFE_FOR, true) < 0) {
RPEDEBUG("Failed concatenating regular expression string");
talloc_free(regmatch);
return XLAT_ACTION_FAIL;
Expand Down Expand Up @@ -703,7 +703,7 @@ static xlat_action_t xlat_regex_resume(TALLOC_CTX *ctx, fr_dcursor_t *out,
* concatenate it here. We escape the various untrusted inputs.
*/
if (fr_value_box_list_concat_as_string(NULL, NULL, agg, &rctx->list, NULL, 0, &regex_escape_rules,
FR_VALUE_BOX_LIST_FREE_BOX, true) < 0) {
FR_VALUE_BOX_LIST_FREE_BOX, FR_REGEX_SAFE_FOR, true) < 0) {
RPEDEBUG("Failed concatenating regular expression string");
return XLAT_ACTION_FAIL;
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/unlang/xlat_func.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ void xlat_func_resolve_set(xlat_t *xlat, xlat_resolve_t func);
void xlat_purify_func_set(xlat_t *xlat, xlat_purify_t func);

/** Set the escaped values for output boxes
*
* Any boxes output by the xlat function will have their values marked as safe for something.
*
* @param[in] _xlat function to set the escaped value for (as returned by xlat_register).
* @param[in] _escaped escaped value to write to output boxes.
Expand Down
4 changes: 4 additions & 0 deletions src/lib/util/regex.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ typedef struct {

#define REGEX_FLAG_BUFF_SIZE 7

/** Unique safefor value to prevent escaping for regexes
*/
#define FR_REGEX_SAFE_FOR ((uintptr_t)regex_exec)

ssize_t regex_flags_parse(int *err, fr_regex_flags_t *out, fr_sbuff_t *in,
fr_sbuff_term_t const *terminals, bool err_on_dup);

Expand Down
17 changes: 9 additions & 8 deletions src/lib/util/value.c
Original file line number Diff line number Diff line change
Expand Up @@ -5468,7 +5468,7 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff
FR_SBUFF_RETURN(fr_value_box_list_concat_as_string,
NULL, NULL, &our_out, UNCONST(fr_value_box_list_t *, &data->vb_group),
", ", (sizeof(", ") - 1), e_rules,
0, false);
0, 0, false);
FR_SBUFF_IN_CHAR_RETURN(&our_out, '}');
break;

Expand Down Expand Up @@ -5536,6 +5536,7 @@ ssize_t fr_value_box_print_quoted(fr_sbuff_t *out, fr_value_box_t const *data, f
* Is not currently applied to any other box type.
* @param[in] proc_action What to do with the boxes in the list once
* they've been processed.
* @param[in] safe_for if value has this safe_for value, don't apply the escape rules.
* @param[in] flatten If true and we encounter a #FR_TYPE_GROUP,
* we concat the contents of its children together.
* If false, the contents will be cast to #FR_TYPE_STRING.
Expand All @@ -5546,7 +5547,7 @@ ssize_t fr_value_box_print_quoted(fr_sbuff_t *out, fr_value_box_t const *data, f
*/
ssize_t fr_value_box_list_concat_as_string(bool *tainted, bool *secret, fr_sbuff_t *sbuff, fr_value_box_list_t *list,
char const *sep, size_t sep_len, fr_sbuff_escape_rules_t const *e_rules,
fr_value_box_list_action_t proc_action, bool flatten)
fr_value_box_list_action_t proc_action, fr_value_box_safe_for_t safe_for, bool flatten)
{
fr_sbuff_t our_sbuff = FR_SBUFF(sbuff);
ssize_t slen;
Expand All @@ -5559,23 +5560,23 @@ ssize_t fr_value_box_list_concat_as_string(bool *tainted, bool *secret, fr_sbuff
if (!flatten) goto print;
slen = fr_value_box_list_concat_as_string(tainted, secret, &our_sbuff, &vb->vb_group,
sep, sep_len, e_rules,
proc_action, flatten);
proc_action, safe_for, flatten);
break;

case FR_TYPE_OCTETS:

/*
* Copy the raw string over, if necessary with escaping.
*/
if (e_rules && (vb->tainted || e_rules->do_oct || e_rules->do_hex)) {
if (e_rules && (!fr_value_box_is_safe_for(vb, safe_for) || e_rules->do_oct || e_rules->do_hex)) {
slen = fr_sbuff_in_escape(&our_sbuff, (char const *)vb->vb_strvalue, vb->vb_length, e_rules);
} else {
slen = fr_sbuff_in_bstrncpy(&our_sbuff, (char const *)vb->vb_strvalue, vb->vb_length);
}
break;

case FR_TYPE_STRING:
if (vb->tainted && e_rules) goto print;
if (!fr_value_box_is_safe_for(vb, safe_for) && e_rules) goto print;

slen = fr_sbuff_in_bstrncpy(&our_sbuff, vb->vb_strvalue, vb->vb_length);
break;
Expand Down Expand Up @@ -5798,7 +5799,7 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx,
*/
if (fr_value_box_list_concat_as_string(&tainted, &secret, &sbuff, list,
NULL, 0, NULL,
FR_VALUE_BOX_LIST_REMOVE, flatten) < 0) {
FR_VALUE_BOX_LIST_REMOVE, 0, flatten) < 0) {
fr_strerror_printf("Concatenation exceeded max_size (%zu)", max_size);
error:
switch (type) {
Expand All @@ -5821,7 +5822,7 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx,
*/
if (fr_value_box_list_concat_as_string(&tainted, &secret, &sbuff, list,
NULL, 0, NULL,
proc_action, flatten) < 0) {
proc_action, 0, flatten) < 0) {
fr_value_box_list_insert_head(list, head_vb);
goto error;
}
Expand Down Expand Up @@ -5863,7 +5864,7 @@ int fr_value_box_list_concat_in_place(TALLOC_CTX *ctx,
case FR_TYPE_STRING:
if (fr_value_box_list_concat_as_string(&tainted, &secret, &sbuff, list,
NULL, 0, NULL,
proc_action, flatten) < 0) goto error;
proc_action, 0, flatten) < 0) goto error;
(void)fr_sbuff_trim_talloc(&sbuff, SIZE_MAX);

entry = out->entry;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/util/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ ssize_t fr_value_box_from_str(TALLOC_CTX *ctx, fr_value_box_t *dst,
*/
ssize_t fr_value_box_list_concat_as_string(bool *tainted, bool *secret, fr_sbuff_t *sbuff, fr_value_box_list_t *list,
char const *sep, size_t sep_len, fr_sbuff_escape_rules_t const *e_rules,
fr_value_box_list_action_t proc_action, bool flatten)
fr_value_box_list_action_t proc_action, fr_value_box_safe_for_t safe_for, bool flatten)
CC_HINT(nonnull(3,4));

ssize_t fr_value_box_list_concat_as_octets(bool *tainted, bool *secret, fr_dbuff_t *dbuff, fr_value_box_list_t *list,
Expand Down
8 changes: 6 additions & 2 deletions src/tests/keywords/regex-escape
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@ string test_string2
# Strings which are expanded in a regex have regex special
# characters escaped. Because the input strings are unsafe.
#
test_string1 := "%taint(example.com)"
test_string2 := "%taint(exampleXcom)"
test_string1 := "example.com"
test_string2 := "exampleXcom"

if ("exampleXcom" =~ /%{test_string1}/) {
test_fail
}

if ("exampleXcom" !~ /%regex.safe(%{test_string1})/) {
test_fail
}

if (test_string2 =~ /%{test_string1}/) {
test_fail
}
Expand Down

0 comments on commit bc1b922

Please sign in to comment.