diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c index b89d3e4..19f70d1 100644 --- a/modules/mappers/mod_rewrite.c +++ b/modules/mappers/mod_rewrite.c @@ -166,6 +166,7 @@ static const char* really_last_key = "rewrite_really_last"; #define RULEFLAG_DISCARDPATHINFO 1<<15 #define RULEFLAG_QSDISCARD 1<<16 #define RULEFLAG_END 1<<17 +#define RULEFLAG_QSNONE (1<<20) /* programattic only */ /* return code of the rewrite rule * the result may be escaped - or not @@ -725,10 +726,18 @@ static char *escape_absolute_uri(apr_pool_t *p, char *uri, unsigned scheme) * split out a QUERY_STRING part from * the current URI string */ -static void splitout_queryargs(request_rec *r, int qsappend, int qsdiscard) +static void splitout_queryargs(request_rec *r, int flags) { char *q; int split; + int qsappend = flags & RULEFLAG_QSAPPEND; + int qsdiscard = flags & RULEFLAG_QSDISCARD; + + if (flags & RULEFLAG_QSNONE) { + rewritelog((r, 2, NULL, "discarding query string, no parse from substitution")); + r->args = NULL; + return; + } /* don't touch, unless it's a scheme for which a query string makes sense. * See RFC 1738 and RFC 2368. @@ -2661,7 +2670,7 @@ static apr_status_t rewritelock_remove(void *data) * XXX: what an inclined parser. Seems we have to leave it so * for backwards compat. *sigh* */ -static int parseargline(char *str, char **a1, char **a2, char **a3) +static int parseargline(char *str, char **a1, char **a2, char **a2_end, char **a3) { char quote; @@ -2712,8 +2721,10 @@ static int parseargline(char *str, char **a1, char **a2, char **a3) if (!*str) { *a3 = NULL; /* 3rd argument is optional */ + *a2_end = str; return 0; } + *a2_end = str; *str++ = '\0'; while (apr_isspace(*str)) { @@ -3230,6 +3241,7 @@ static const char *cmd_rewritecond(cmd_parms *cmd, void *in_dconf, ap_regex_t *regexp; char *a1; char *a2; + char *a2_end; char *a3; const char *err; @@ -3248,7 +3260,7 @@ static const char *cmd_rewritecond(cmd_parms *cmd, void *in_dconf, * of the argument line. So we can use a1 .. a3 without * copying them again. */ - if (parseargline(str, &a1, &a2, &a3)) { + if (parseargline(str, &a1, &a2, &a2_end, &a3)) { return apr_pstrcat(cmd->pool, "RewriteCond: bad argument line '", str, "'", NULL); } @@ -3645,6 +3657,7 @@ static const char *cmd_rewriterule(cmd_parms *cmd, void *in_dconf, ap_regex_t *regexp; char *a1; char *a2; + char *a2_end; char *a3; const char *err; @@ -3659,7 +3672,7 @@ static const char *cmd_rewriterule(cmd_parms *cmd, void *in_dconf, } /* parse the argument line ourself */ - if (parseargline(str, &a1, &a2, &a3)) { + if (parseargline(str, &a1, &a2, &a2_end, &a3)) { return apr_pstrcat(cmd->pool, "RewriteRule: bad argument line '", str, "'", NULL); } @@ -3705,6 +3718,16 @@ static const char *cmd_rewriterule(cmd_parms *cmd, void *in_dconf, newrule->flags |= RULEFLAG_NOSUB; } + if (*(a2_end-1) == '?') { + /* a literal ? at the end of the unsubstituted rewrite rule */ + newrule->flags |= RULEFLAG_QSNONE; + } + else if (newrule->flags & RULEFLAG_QSDISCARD) { + if (NULL == ap_strchr(newrule->output, '?')) { + newrule->flags |= RULEFLAG_QSNONE; + } + } + /* now, if the server or per-dir config holds an * array of RewriteCond entries, we take it for us * and clear the array @@ -4110,7 +4133,7 @@ static int apply_rewrite_rule(rewriterule_entry *p, rewrite_ctx *ctx) r->path_info = NULL; } - splitout_queryargs(r, p->flags & RULEFLAG_QSAPPEND, p->flags & RULEFLAG_QSDISCARD); + splitout_queryargs(r, p->flags); /* Add the previously stripped per-directory location prefix, unless * (1) it's an absolute URL path and @@ -4565,6 +4588,17 @@ static int hook_uri2file(request_rec *r) unsigned skip; apr_size_t flen; + if (r->args && *(ap_scan_vchar_obstext(r->args))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10410) + "Rewritten query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } + if (ACTION_STATUS == rulestatus) { int n = r->status; @@ -4833,6 +4867,17 @@ static int hook_fixup(request_rec *r) if (rulestatus) { unsigned skip; + if (r->args && *(ap_scan_vchar_obstext(r->args))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10411) + "Rewritten query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } + if (ACTION_STATUS == rulestatus) { int n = r->status; diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index 9b69a2e..a52d777 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -69,6 +69,16 @@ static int proxy_ajp_canon(request_rec *r, char *url) path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0, r->proxyreq); search = r->args; + if (search && *(ap_scan_vchar_obstext(search))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10406) + "To be forwarded query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } } if (path == NULL) return HTTP_BAD_REQUEST; diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index 4d9d2af..fa385c3 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -94,6 +94,16 @@ static int proxy_balancer_canon(request_rec *r, char *url) path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0, r->proxyreq); search = r->args; + if (search && *(ap_scan_vchar_obstext(search))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10407) + "To be forwarded query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } } if (path == NULL) return HTTP_BAD_REQUEST; diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 6767c89..1a10d99 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -87,6 +87,16 @@ static int proxy_http_canon(request_rec *r, char *url) path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0, r->proxyreq); search = r->args; + if (search && *(ap_scan_vchar_obstext(search))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10408) + "To be forwarded query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } } break; case PROXYREQ_PROXY: diff --git a/modules/proxy/mod_proxy_wstunnel.c b/modules/proxy/mod_proxy_wstunnel.c index eb34eee..438d035 100644 --- a/modules/proxy/mod_proxy_wstunnel.c +++ b/modules/proxy/mod_proxy_wstunnel.c @@ -73,6 +73,16 @@ static int proxy_wstunnel_canon(request_rec *r, char *url) path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0, r->proxyreq); search = r->args; + if (search && *(ap_scan_vchar_obstext(search))) { + /* + * We have a raw control character or a ' ' in r->args. + * Correct encoding was missed. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10409) + "To be forwarded query string contains control " + "characters or spaces"); + return HTTP_FORBIDDEN; + } } if (path == NULL) return HTTP_BAD_REQUEST;