List Info

Thread: Re: svn commit: r571414 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_connect.c




Re: svn commit: r571414 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_connect.c
user name
2007-09-01 11:58:29

On 08/31/2007 12:14 PM, wrote:
> Author: niq
> Date: Fri Aug 31 03:13:59 2007
> New Revision: 571414
> 
> URL: 
http://svn.apache.org/viewvc?rev=571414&view=rev
> Log:
> Fix potential proxy segfault
> PR 40756.  Patch adapted from 2.0 patch by Trevin
Beattie
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>    
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> 
> Modified: httpd/httpd/trunk/CHANGES

> Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> URL: http://svn.apache.org/viewvc/httpd/http
d/trunk/modules/proxy/mod_proxy_connect.c?rev=571414&r1=
571413&r2=571414&view=diff
>
============================================================
==================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
(original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
Fri Aug 31 03:13:59 2007
>  -125,18 +125,24 
>  
>      /* are we connecting directly, or via a proxy? */
>      if (proxyname) {
> -    connectname = proxyname;
> -    connectport = proxyport;
> +        connectname = proxyname;
> +        connectport = proxyport;
>          err = apr_sockaddr_info_get(&connect_addr,
proxyname, APR_UNSPEC, proxyport, 0, p);
>      }
>      else {
> -    connectname = uri.hostname;
> -    connectport = uri.port;
> -    connect_addr = uri_addr;
> +        connectname = uri.hostname;
> +        connectport = uri.port;
> +        connect_addr = uri_addr;
>      }
>      ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
r->server,
>           "proxy: CONNECT: connecting to remote
proxy %s on port %d", connectname, connectport);
>  
> +    if (APR_SUCCESS != err) {
> +	return ap_proxyerror(r, HTTP_BAD_GATEWAY,
apr_pstrcat(p,
> +			     "DNS lookup failure for: ",
> +			     uri.hostname, NULL));
> +    }
> + 
>      /* check if ProxyBlock directive on this host */
>      if (OK != ap_proxy_checkproxyblock(r, conf,
uri_addr)) {

Hm. In the case that apr_sockaddr_info_get fails for
uri.hostname, but succeeds for
proxyname (if we have one) we still crash in
ap_proxy_checkproxyblock, don't we?
Because in this case err will be reset to APR_SUCCESS by the
second apr_sockaddr_info_get
call.

Regards

RĂ¼diger

Re: svn commit: r571414 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_connect.c
country flaguser name
United States
2007-09-01 12:59:52
Ruediger Pluem wrote:
> 
> 
> 
> On 08/31/2007 12:14 PM, wrote:
> > Author: niq
> > Date: Fri Aug 31 03:13:59 2007
> > New Revision: 571414
> > 
> > URL: 
http://svn.apache.org/viewvc?rev=571414&view=rev
> > Log:
> > Fix potential proxy segfault
> > PR 40756.  Patch adapted from 2.0 patch by Trevin
Beattie
> > 
> > Modified:
> >     httpd/httpd/trunk/CHANGES
> >    
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> > 
> > Modified: httpd/httpd/trunk/CHANGES
> 
> > Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> > URL: http://svn.apache.org/viewvc/httpd/http
d/trunk/modules/proxy/mod_proxy_connect.c?rev=571414&r1=
571413&r2=571414&view=diff
> >
============================================================
==================
> > ---
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
(original)
> > +++
httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c Fri Aug
31 03:13:59 2007
> >  -125,18 +125,24 
> >  
> >      /* are we connecting directly, or via a
proxy? */
> >      if (proxyname) {
> > -    connectname = proxyname;
> > -    connectport = proxyport;
> > +        connectname = proxyname;
> > +        connectport = proxyport;
> >          err =
apr_sockaddr_info_get(&connect_addr, proxyname,
APR_UNSPEC, proxyport, 0, p);
> >      }
> >      else {
> > -    connectname = uri.hostname;
> > -    connectport = uri.port;
> > -    connect_addr = uri_addr;
> > +        connectname = uri.hostname;
> > +        connectport = uri.port;
> > +        connect_addr = uri_addr;
> >      }
> >      ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
r->server,
> >           "proxy: CONNECT: connecting to
remote proxy %s on port %d", connectname,
connectport);
> >  
> > +    if (APR_SUCCESS != err) {
> > +	return ap_proxyerror(r, HTTP_BAD_GATEWAY,
apr_pstrcat(p,
> > +			     "DNS lookup failure for: ",
> > +			     uri.hostname, NULL));
> > +    }
> > + 
> >      /* check if ProxyBlock directive on this host
*/
> >      if (OK != ap_proxy_checkproxyblock(r, conf,
uri_addr)) {
> 
> Hm. In the case that apr_sockaddr_info_get fails for
uri.hostname, but succeeds for
> proxyname (if we have one) we still crash in
ap_proxy_checkproxyblock, don't we?
> Because in this case err will be reset to APR_SUCCESS
by the second apr_sockaddr_info_get
> call.
> 

I need to look (stepping out in a few mins) but I think you
are right.

Plus, nit-picking time, there are other lines in this file
that could use reformating 

-- 
============================================================
===============
   Jim Jagielski   [|]   jimjaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a
ball."

[1-2]

about | contact  Other archives ( Real Estate discussion Medical topics )