You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2023/03/31 00:11:03 UTC
svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Author: ylavic
Date: Fri Mar 31 00:11:02 2023
New Revision: 1908827
URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
Log:
mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
Modified:
httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
Modified: httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/mod_proxy_http2.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http2/mod_proxy_http2.c (original)
+++ httpd/httpd/trunk/modules/http2/mod_proxy_http2.c Fri Mar 31 00:11:02 2023
@@ -164,26 +164,31 @@ static int proxy_http2_canon(request_rec
path = ap_proxy_canonenc_ex(r->pool, url, (int)strlen(url),
enc_path, flags, r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
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(10412)
- "To be forwarded query string contains control "
- "characters or spaces");
- return HTTP_FORBIDDEN;
- }
break;
case PROXYREQ_PROXY:
path = url;
break;
}
-
- if (path == NULL) {
- return HTTP_BAD_REQUEST;
+ /*
+ * If we have a raw control character or a ' ' in nocanon path or
+ * r->args, correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10420)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
+ if (search && *ap_scan_vchar_obstext(search)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10412)
+ "To be forwarded query string contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
}
if (port != def_port) {
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Fri Mar 31 00:11:02 2023
@@ -75,20 +75,27 @@ static int proxy_ajp_canon(request_rec *
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
search = r->args;
}
+ /*
+ * If we have a raw control character or a ' ' in nocanon path or
+ * r->args, correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10418)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
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;
if (port != def_port)
apr_snprintf(sport, sizeof(sport), ":%d", port);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Mar 31 00:11:02 2023
@@ -112,20 +112,27 @@ static int proxy_balancer_canon(request_
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
search = r->args;
}
+ /*
+ * If we have a raw control character or a ' ' in nocanon path or
+ * r->args, correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10416)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
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;
r->filename = apr_pstrcat(r->pool, "proxy:" BALANCER_PREFIX, host,
"/", path, (search) ? "?" : "", (search) ? search : "", NULL);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Fri Mar 31 00:11:02 2023
@@ -102,9 +102,20 @@ static int proxy_fcgi_canon(request_rec
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
+ }
+ /*
+ * If we have a raw control character or a ' ' in nocanon path,
+ * correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10414)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
}
- if (path == NULL)
- return HTTP_BAD_REQUEST;
r->filename = apr_pstrcat(r->pool, "proxy:fcgi://", host, sport, "/",
path, NULL);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Fri Mar 31 00:11:02 2023
@@ -128,26 +128,32 @@ static int proxy_http_canon(request_rec
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path,
flags, r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
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:
path = url;
break;
}
-
- if (path == NULL)
- return HTTP_BAD_REQUEST;
+ /*
+ * If we have a raw control character or a ' ' in nocanon path or
+ * r->args, correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10415)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
+ if (search && *ap_scan_vchar_obstext(search)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10408)
+ "To be forwarded query string contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
if (port != def_port)
apr_snprintf(sport, sizeof(sport), ":%d", port);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c Fri Mar 31 00:11:02 2023
@@ -94,9 +94,19 @@ static int uwsgi_canon(request_rec *r, c
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
}
- if (!path) {
- return HTTP_BAD_REQUEST;
+ /*
+ * If we have a raw control character or a ' ' in nocanon path,
+ * correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10417)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
}
r->filename =
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c?rev=1908827&r1=1908826&r2=1908827&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Fri Mar 31 00:11:02 2023
@@ -205,20 +205,27 @@ static int proxy_wstunnel_canon(request_
path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
r->proxyreq);
+ if (!path) {
+ return HTTP_BAD_REQUEST;
+ }
search = r->args;
}
+ /*
+ * If we have a raw control character or a ' ' in nocanon path or
+ * r->args, correct encoding was missed.
+ */
+ if (path == url && *ap_scan_vchar_obstext(path)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10419)
+ "To be forwarded path contains control "
+ "characters or spaces");
+ return HTTP_FORBIDDEN;
+ }
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;
if (port != def_port)
apr_snprintf(sport, sizeof(sport), ":%d", port);
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 11:23 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 10:25 AM, Yann Ylavic wrote:
> > On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 3/31/23 9:56 AM, Yann Ylavic wrote:
> >>> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>>>
> >>>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> >>>>> Author: ylavic
> >>>>> Date: Fri Mar 31 00:11:02 2023
> >>>>> New Revision: 1908827
> >>>>>
> >>>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> >>>>> Log:
> >>>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
> >>>>
> >>>> Can this really happen? Wouldn't this path be rejected during request line parsing?
> >>>
> >>> This is on the suspenders plus belt side (check what we send), but I
> >>> suppose that an unfortunate NE[,P] rule could cause it.
> >>
> >> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
> >
> > NE,P forces nocanon in mod_rewrite for instance.
>
> Good catch. I missed this.
I was tempted to not do it for "noencode" (i.e. mapping=encoded)
because it is the (most-)fast path for proxying, likely w/o
RewriteRules or the users really knowing what they are doing here, but
finally went with common code. I could be convinced though :)
>
> >
> >> And if we think that this is a possibility shouldn't we check
> >>
> >> *ap_scan_vchar_obstext(r->filename)
> >>
> >> just before returning, to be really sure we missed no edge case?
> >
> > We could do that, but the path and search parts of the constructed
> > r->filename seem to be the only ones we haven't "parsed" in
> > canon_handler/ap_proxy_canon_netloc already.
>
> Ok, fair enough. Do you care to propose for backport such that we get it in before the tag?
Done (STATUS + backport PR #354).
Thanks;
Yann.
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/31/23 10:25 AM, Yann Ylavic wrote:
> On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 3/31/23 9:56 AM, Yann Ylavic wrote:
>>> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>>>>
>>>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
>>>>> Author: ylavic
>>>>> Date: Fri Mar 31 00:11:02 2023
>>>>> New Revision: 1908827
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
>>>>> Log:
>>>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>>>>
>>>> Can this really happen? Wouldn't this path be rejected during request line parsing?
>>>
>>> This is on the suspenders plus belt side (check what we send), but I
>>> suppose that an unfortunate NE[,P] rule could cause it.
>>
>> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
>
> NE,P forces nocanon in mod_rewrite for instance.
Good catch. I missed this.
>
>> And if we think that this is a possibility shouldn't we check
>>
>> *ap_scan_vchar_obstext(r->filename)
>>
>> just before returning, to be really sure we missed no edge case?
>
> We could do that, but the path and search parts of the constructed
> r->filename seem to be the only ones we haven't "parsed" in
> canon_handler/ap_proxy_canon_netloc already.
Ok, fair enough. Do you care to propose for backport such that we get it in before the tag?
Regards
Rüdiger
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 9:56 AM, Yann Ylavic wrote:
> > On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> >>> Author: ylavic
> >>> Date: Fri Mar 31 00:11:02 2023
> >>> New Revision: 1908827
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> >>> Log:
> >>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
> >>
> >> Can this really happen? Wouldn't this path be rejected during request line parsing?
> >
> > This is on the suspenders plus belt side (check what we send), but I
> > suppose that an unfortunate NE[,P] rule could cause it.
>
> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
NE,P forces nocanon in mod_rewrite for instance.
> And if we think that this is a possibility shouldn't we check
>
> *ap_scan_vchar_obstext(r->filename)
>
> just before returning, to be really sure we missed no edge case?
We could do that, but the path and search parts of the constructed
r->filename seem to be the only ones we haven't "parsed" in
canon_handler/ap_proxy_canon_netloc already.
Regards;
Yann.
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/31/23 9:56 AM, Yann Ylavic wrote:
> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
>>> Author: ylavic
>>> Date: Fri Mar 31 00:11:02 2023
>>> New Revision: 1908827
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
>>> Log:
>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>>
>> Can this really happen? Wouldn't this path be rejected during request line parsing?
>
> This is on the suspenders plus belt side (check what we send), but I
> suppose that an unfortunate NE[,P] rule could cause it.
Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
And if we think that this is a possibility shouldn't we check
*ap_scan_vchar_obstext(r->filename)
just before returning, to be really sure we missed no edge case?
Regards
Rüdiger
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Fri Mar 31 00:11:02 2023
> > New Revision: 1908827
> >
> > URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> > Log:
> > mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>
> Can this really happen? Wouldn't this path be rejected during request line parsing?
This is on the suspenders plus belt side (check what we send), but I
suppose that an unfortunate NE[,P] rule could cause it.
Regards;
Yann.
Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Fri Mar 31 00:11:02 2023
> New Revision: 1908827
>
> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> Log:
> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
Can this really happen? Wouldn't this path be rejected during request line parsing?
Regards
Rüdiger