You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2021/02/02 19:50:14 UTC
svn commit: r1886141 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Author: rpluem
Date: Tue Feb 2 19:50:14 2021
New Revision: 1886141
URL: http://svn.apache.org/viewvc?rev=1886141&view=rev
Log:
* We need to check for (!scheme && (u = strchr(url, ':')) && (u - url) > 14)
later as (!scheme || u[0] != '/' || u[1] != '/' || u[2] == '\0') is true
for requests with the CONNECT method which we need to decline. But in many
cases requests with the CONNECT method have (u - url) > 14 as in this
case (u - url) is the length of the FQDN the forward proxy should connect
to.
Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
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=1886141&r1=1886140&r2=1886141&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Feb 2 19:50:14 2021
@@ -1903,15 +1903,15 @@ static int proxy_http_handler(request_re
is_ssl = 0;
}
if (!scheme || u[0] != '/' || u[1] != '/' || u[2] == '\0') {
- if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
- "overlong proxy URL scheme in %s", url);
- return HTTP_BAD_REQUEST;
- }
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01113)
"HTTP: declining URL %s", url);
return DECLINED; /* only interested in HTTP, WS or FTP via proxy */
}
+ if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
+ "overlong proxy URL scheme in %s", url);
+ return HTTP_BAD_REQUEST;
+ }
if (is_ssl && !ap_proxy_ssl_enable(NULL)) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01112)
"HTTP: declining URL %s (mod_ssl not configured?)", url);
Re: svn commit: r1886141 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Feb 3, 2021 at 9:48 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 2/3/21 1:23 AM, Yann Ylavic wrote:
> >
> > I think we can remove this test/block completely now that
>
> Agreed. I think we should just decline and leave it to further possible protocol handlers to deal
> with these URL's. And if all decline we just hit the catch all that there is no handler for this URL.
> So +1 to just remove the block. Will you do?
r1886151.
Regards;
Yann.
Re: svn commit: r1886141 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 2/3/21 1:23 AM, Yann Ylavic wrote:
> On Tue, Feb 2, 2021 at 8:50 PM <rp...@apache.org> wrote:
>>
>> Author: rpluem
>> Date: Tue Feb 2 19:50:14 2021
>> New Revision: 1886141
>>
> []
>> if (!scheme || u[0] != '/' || u[1] != '/' || u[2] == '\0') {
>> - if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
>> - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
>> - "overlong proxy URL scheme in %s", url);
>> - return HTTP_BAD_REQUEST;
>> - }
>> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01113)
>> "HTTP: declining URL %s", url);
>> return DECLINED; /* only interested in HTTP, WS or FTP via proxy */
>> }
>> + if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
>> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
>> + "overlong proxy URL scheme in %s", url);
>> + return HTTP_BAD_REQUEST;
>> + }
>
> Hmm, actually this !scheme here is unreachable now.
Good catch.
> I think we can remove this test/block completely now that
Agreed. I think we should just decline and leave it to further possible protocol handlers to deal
with these URL's. And if all decline we just hit the catch all that there is no handler for this URL.
So +1 to just remove the block. Will you do?
Regards
Rüdiger
Re: svn commit: r1886141 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Feb 2, 2021 at 8:50 PM <rp...@apache.org> wrote:
>
> Author: rpluem
> Date: Tue Feb 2 19:50:14 2021
> New Revision: 1886141
>
[]
> if (!scheme || u[0] != '/' || u[1] != '/' || u[2] == '\0') {
> - if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
> - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
> - "overlong proxy URL scheme in %s", url);
> - return HTTP_BAD_REQUEST;
> - }
> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01113)
> "HTTP: declining URL %s", url);
> return DECLINED; /* only interested in HTTP, WS or FTP via proxy */
> }
> + if (!scheme && (u = strchr(url, ':')) && (u - url) > 14) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
> + "overlong proxy URL scheme in %s", url);
> + return HTTP_BAD_REQUEST;
> + }
Hmm, actually this !scheme here is unreachable now.
I think we can remove this test/block completely now that
get_url_scheme() checks the exact scheme.
The old code was like this (removed hunk from r1885239):
- if ((u - url) > 14)
- return HTTP_BAD_REQUEST;
- scheme = apr_pstrmemdup(p, url, u - url);
and probably meant to avoid allocating an unreasonable scheme, but
thanks to get_url_scheme() we do not allocate anymore, so this check
is probably useless now.
It's quite hard to reject overlong schemes in mod_proxy_http anyway,
because it'll really take the !scheme branch above, and since a scheme
is [[:alnum:].+-] it may well be a hostname too and match a CONNECT
URI.
So should we check for:
if (!scheme || u[0] != '/' || u[1] != '/' || u[2] == '\0') {
if (!scheme && (u = strchr(url, ':')) && (u - url) > 14 &&
!apr_isdigit(u[0])) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10262)
"overlong proxy URL scheme in %s", url);
return HTTP_BAD_REQUEST;
}
return DECLINED; /* only interested in HTTP, WS or FTP via proxy */
}
or something?
Regards;
Yann.