Re: svn commit: r1688399 - /httpd/httpd/trunk/modules/metadata/mod_remoteip.c

[Eric Covener <co...@gmail.com>]

This was not backported and popped up in PR60251.

Bill, can you have a look including my guess that it really should
just be "temp_sa = r->useragent_addr;"?

On Tue, Jun 30, 2015 at 4:40 AM,  <jk...@apache.org> wrote:
> Author: jkaluza
> Date: Tue Jun 30 08:40:17 2015
> New Revision: 1688399
>
> URL: http://svn.apache.org/r1688399
> Log:
> mod_remoteip: Use r->useragent_addr as the root trusted address for verifying.
>
> This fixes issue resulting in setting of bad useragent_ip when internal
> redirection has been generated as response to the request (typically as
> result of "ErrorDocument 40x").
>
> In this case, the original request has been handled by mod_remoteip and its
> useragent_ip has been changed properly, but when internal redirection
> to ErrorDocument has been generated later, the mod_remoteip's handler has been
> executed again with *the same* c->client_addr as in the original request. If
> c->client_addr IP is trusted, this results in bad useragent_ip being set.
>
> When using r->useragent_addr as the root trusted address instead of
> c->client_addr, the internal redirection uses the first non-trusted
> IP in this particular case, so it won't change the r->useragent_ip during
> the internal redirection to ErrorDocument.
>
> Modified:
>     httpd/httpd/trunk/modules/metadata/mod_remoteip.c
>
> Modified: httpd/httpd/trunk/modules/metadata/mod_remoteip.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_remoteip.c?rev=1688399&r1=1688398&r2=1688399&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original)
> +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Tue Jun 30 08:40:17 2015
> @@ -255,7 +255,7 @@ static int remoteip_modify_request(reque
>      }
>      remote = apr_pstrdup(r->pool, remote);
>
> -    temp_sa = c->client_addr;
> +    temp_sa = r->useragent_addr ? r->useragent_addr : c->client_addr;
>
>      while (remote) {
>
>
>



-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1688399 - /httpd/httpd/trunk/modules/metadata/mod_remoteip.c

[William A Rowe Jr <wr...@rowe-clan.net>]

On Fri, Oct 14, 2016 at 11:16 AM, Eric Covener <co...@gmail.com> wrote:

> This was not backported and popped up in PR60251.
>
> Bill, can you have a look including my guess that it really should
> just be "temp_sa = r->useragent_addr;"?


While that code should *not* be triggered before r->useragent_addr
has been populated, some off-beat perl code causes these phases
to run out-of-sequence and we segfault not long after if this is run
without a post read request hook.

I blame a bad mod_perl example, but the cycle wasted to confirm
that useragent_addr is non-null isn't worth trimming.