243 lines
8.8 KiB
Diff
243 lines
8.8 KiB
Diff
From a0a68b99d131741c1867cff321424892838fc4b3 Mon Sep 17 00:00:00 2001
|
|
From: Yann Ylavic <ylavic@apache.org>
|
|
Date: Sat, 27 Jul 2024 13:35:53 +0000
|
|
Subject: [PATCH] mod_rewrite: Better question mark tracking to avoid
|
|
UnsafeAllow3F. PR 69197.
|
|
|
|
Track in do_expand() whether a '?' in the uri-path comes from a literal in
|
|
the substitution string or from an expansion (variable, lookup, ...).
|
|
In the former case it's safe to assume that it's the query-string separator
|
|
but for the other case it's not (could be a decoded %3f from r->uri).
|
|
|
|
This allows to avoid [UnsafeAllow3F] for most cases.
|
|
|
|
|
|
Merges r1919325 from trunk
|
|
Reviewed by: ylavic, covener, jorton
|
|
Github: closes #462
|
|
|
|
|
|
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1919545 13f79535-47bb-0310-9956-ffa450edef68
|
|
---
|
|
modules/mappers/mod_rewrite.c | 107 ++++++++++++++++++++++++++++------
|
|
1 file changed, 89 insertions(+), 18 deletions(-)
|
|
|
|
|
|
diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c
|
|
index 226602f..a56a85c 100644
|
|
--- a/modules/mappers/mod_rewrite.c
|
|
+++ b/modules/mappers/mod_rewrite.c
|
|
@@ -2365,9 +2365,16 @@ static APR_INLINE char *find_char_in_curlies(char *s, int c)
|
|
* of an earlier expansion to include expansion specifiers that
|
|
* are interpreted by a later expansion, producing results that
|
|
* were not intended by the administrator.
|
|
+ *
|
|
+ * unsafe_qmark if not NULL will be set to 1 or 0 if a question mark
|
|
+ * is found respectively in a literal or in a lookup/expansion (whether
|
|
+ * it's the first or last qmark depends on [QSL]). Should be initialized
|
|
+ * to -1 and remains so if no qmark is found.
|
|
*/
|
|
-static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
+static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry,
|
|
+ int *unsafe_qmark)
|
|
{
|
|
+#define EXPAND_SPECIALS "\\$%"
|
|
result_list *result, *current;
|
|
result_list sresult[SMALL_EXPANSION];
|
|
unsigned spc = 0;
|
|
@@ -2375,8 +2382,29 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
char *p, *c;
|
|
apr_pool_t *pool = ctx->r->pool;
|
|
|
|
- span = strcspn(input, "\\$%");
|
|
inputlen = strlen(input);
|
|
+ if (!unsafe_qmark) {
|
|
+ span = strcspn(input, EXPAND_SPECIALS);
|
|
+ }
|
|
+ else {
|
|
+ span = strcspn(input, EXPAND_SPECIALS "?");
|
|
+ if (input[span] == '?') {
|
|
+ /* this qmark is not from an expansion thus safe */
|
|
+ *unsafe_qmark = 0;
|
|
+
|
|
+ /* keep tracking only if interested in the last qmark */
|
|
+ if (entry && (entry->flags & RULEFLAG_QSLAST)) {
|
|
+ do {
|
|
+ span++;
|
|
+ span += strcspn(input + span, EXPAND_SPECIALS "?");
|
|
+ } while (input[span] == '?');
|
|
+ }
|
|
+ else {
|
|
+ unsafe_qmark = NULL;
|
|
+ span += strcspn(input + span, EXPAND_SPECIALS);
|
|
+ }
|
|
+ }
|
|
+ }
|
|
|
|
/* fast exit */
|
|
if (inputlen == span) {
|
|
@@ -2394,6 +2422,8 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
|
|
/* loop for specials */
|
|
do {
|
|
+ int expanded = 0;
|
|
+
|
|
/* prepare next entry */
|
|
if (current->len) {
|
|
current->next = (spc < SMALL_EXPANSION)
|
|
@@ -2439,6 +2469,8 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
current->len = span;
|
|
current->string = p;
|
|
outlen += span;
|
|
+
|
|
+ expanded = 1;
|
|
p = endp + 1;
|
|
}
|
|
|
|
@@ -2478,19 +2510,18 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
}
|
|
|
|
/* reuse of key variable as result */
|
|
- key = lookup_map(ctx->r, map, do_expand(key, ctx, entry));
|
|
-
|
|
+ key = lookup_map(ctx->r, map, do_expand(key, ctx, entry, NULL));
|
|
if (!key && dflt && *dflt) {
|
|
- key = do_expand(dflt, ctx, entry);
|
|
+ key = do_expand(dflt, ctx, entry, NULL);
|
|
}
|
|
-
|
|
- if (key) {
|
|
+ if (key && *key) {
|
|
span = strlen(key);
|
|
current->len = span;
|
|
current->string = key;
|
|
outlen += span;
|
|
}
|
|
|
|
+ expanded = 1;
|
|
p = endp + 1;
|
|
}
|
|
}
|
|
@@ -2520,8 +2551,9 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
current->len = span;
|
|
current->string = bri->source + bri->regmatch[n].rm_so;
|
|
}
|
|
-
|
|
outlen += span;
|
|
+
|
|
+ expanded = 1;
|
|
}
|
|
|
|
p += 2;
|
|
@@ -2534,8 +2566,41 @@ static char *do_expand(char *input, rewrite_ctx *ctx, rewriterule_entry *entry)
|
|
++outlen;
|
|
}
|
|
|
|
+ if (unsafe_qmark && expanded && current->len
|
|
+ && memchr(current->string, '?', current->len)) {
|
|
+ /* this qmark is from an expansion thus unsafe */
|
|
+ *unsafe_qmark = 1;
|
|
+
|
|
+ /* keep tracking only if interested in the last qmark */
|
|
+ if (!entry || !(entry->flags & RULEFLAG_QSLAST)) {
|
|
+ unsafe_qmark = NULL;
|
|
+ }
|
|
+ }
|
|
+
|
|
/* check the remainder */
|
|
- if (*p && (span = strcspn(p, "\\$%")) > 0) {
|
|
+ if (!unsafe_qmark) {
|
|
+ span = strcspn(p, EXPAND_SPECIALS);
|
|
+ }
|
|
+ else {
|
|
+ span = strcspn(p, EXPAND_SPECIALS "?");
|
|
+ if (p[span] == '?') {
|
|
+ /* this qmark is not from an expansion thus safe */
|
|
+ *unsafe_qmark = 0;
|
|
+
|
|
+ /* keep tracking only if interested in the last qmark */
|
|
+ if (entry && (entry->flags & RULEFLAG_QSLAST)) {
|
|
+ do {
|
|
+ span++;
|
|
+ span += strcspn(p + span, EXPAND_SPECIALS "?");
|
|
+ } while (p[span] == '?');
|
|
+ }
|
|
+ else {
|
|
+ unsafe_qmark = NULL;
|
|
+ span += strcspn(p + span, EXPAND_SPECIALS);
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+ if (span > 0) {
|
|
if (current->len) {
|
|
current->next = (spc < SMALL_EXPANSION)
|
|
? &(sresult[spc++])
|
|
@@ -2580,7 +2645,7 @@ static void do_expand_env(data_item *env, rewrite_ctx *ctx)
|
|
char *name, *val;
|
|
|
|
while (env) {
|
|
- name = do_expand(env->data, ctx, NULL);
|
|
+ name = do_expand(env->data, ctx, NULL, NULL);
|
|
if (*name == '!') {
|
|
name++;
|
|
apr_table_unset(ctx->r->subprocess_env, name);
|
|
@@ -2714,7 +2779,7 @@ static void add_cookie(request_rec *r, char *s)
|
|
static void do_expand_cookie(data_item *cookie, rewrite_ctx *ctx)
|
|
{
|
|
while (cookie) {
|
|
- add_cookie(ctx->r, do_expand(cookie->data, ctx, NULL));
|
|
+ add_cookie(ctx->r, do_expand(cookie->data, ctx, NULL, NULL));
|
|
cookie = cookie->next;
|
|
}
|
|
|
|
@@ -3992,7 +4057,7 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
|
|
int basis;
|
|
|
|
if (p->ptype != CONDPAT_AP_EXPR)
|
|
- input = do_expand(p->input, ctx, NULL);
|
|
+ input = do_expand(p->input, ctx, NULL, NULL);
|
|
|
|
switch (p->ptype) {
|
|
case CONDPAT_FILE_EXISTS:
|
|
@@ -4156,7 +4221,7 @@ static APR_INLINE void force_type_handler(rewriterule_entry *p,
|
|
char *expanded;
|
|
|
|
if (p->forced_mimetype) {
|
|
- expanded = do_expand(p->forced_mimetype, ctx, p);
|
|
+ expanded = do_expand(p->forced_mimetype, ctx, p, NULL);
|
|
|
|
if (*expanded) {
|
|
ap_str_tolower(expanded);
|
|
@@ -4170,7 +4235,7 @@ static APR_INLINE void force_type_handler(rewriterule_entry *p,
|
|
}
|
|
|
|
if (p->forced_handler) {
|
|
- expanded = do_expand(p->forced_handler, ctx, p);
|
|
+ expanded = do_expand(p->forced_handler, ctx, p, NULL);
|
|
|
|
if (*expanded) {
|
|
ap_str_tolower(expanded);
|
|
@@ -4307,12 +4372,18 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
|
|
|
|
/* expand the result */
|
|
if (!(p->flags & RULEFLAG_NOSUB)) {
|
|
- newuri = do_expand(p->output, ctx, p);
|
|
+ int unsafe_qmark = -1;
|
|
+
|
|
+ if (p->flags & RULEFLAG_UNSAFE_ALLOW3F) {
|
|
+ newuri = do_expand(p->output, ctx, p, NULL);
|
|
+ }
|
|
+ else {
|
|
+ newuri = do_expand(p->output, ctx, p, &unsafe_qmark);
|
|
+ }
|
|
rewritelog((r, 2, ctx->perdir, "rewrite '%s' -> '%s'", ctx->uri,
|
|
newuri));
|
|
- if (!(p->flags & RULEFLAG_UNSAFE_ALLOW3F) &&
|
|
- ap_strcasestr(r->unparsed_uri, "%3f") &&
|
|
- ap_strchr_c(newuri, '?')) {
|
|
+
|
|
+ if (unsafe_qmark > 0) {
|
|
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
|
|
"Unsafe URL with %%3f URL rewritten without "
|
|
"UnsafeAllow3F");
|