Potential security vulnerability #5374

Closed
SteveTalbot opened this Issue Oct 28, 2013 · 4 comments

Comments

Projects
None yet
3 participants

Note: A fix now exists in both master and develop, and has been released with 2.2.5

The X-Forwarded-For header is a comma-separated list of IP
addresses, where the leftmost is the original client and the others
are successive proxies or load balancers. The address of the last
proxy is the apparent source IP address, in

`````` $_SERVER['REMOTE_ADDR']```.

In Zend\Http\PhpEnvironment\RemoteAddress, when $useProxy
is set to true, the getIpAddressFromProxy() function does not
check whether $_SERVER['REMOTE_ADDR'] is one of the trusted
proxies. Hence if the client is not behind a trusted proxy and spoofs
the X-Forwarded-For header, this function will return a spoofed
IP address.

This allows a session hijacking attack because of the way
Zend\Http\PhpEnvironment\RemoteAddress is used in the
Zend\Session\Validator\RemoteAddr session validator.

I think it would be safer to replace the start of the function as follows:


```
protected function getIpAddressFromProxy()
{
    if (!$this->useProxy || !in_array($_SERVER['REMOTE_ADDR'],
```

$this->trustedProxies)) {
            return false;
        }

So if the source IP address is not a trusted proxy,
getIpAddressFromProxy() would return false and
getIpAddress() function would return
$_SERVER['REMOTE_ADDR'], which I think is more desirable
behaviour.

Owner

weierophinney commented Oct 28, 2013

@SteveTalbot Please, never, never, never report potential security issues on a public tracker. Our README clearly details how to report security issues (tl;dr: email to zf-security@zend.com). In the meantime, I've removed the contents of the report until we can review and patch the issue.

Apologies --- to avoid anyone else doing the same, it'd be helpful to add the zf-security e-mail address to https://github.com/zendframework/zf2/blob/master/CONTRIBUTING.md, which is linked from the "New Issue" page on GitHub.

Member

EvanDotPro commented Oct 28, 2013

@SteveTalbot Definitely, we'll be updating the readme and contributing documents to make this as apparent as possible. Thank you.

Owner

weierophinney commented Oct 31, 2013

Patches have been reviewed, and I've applied in my local branch. I'll be pushing shortly, and will release with 2.2.5 later today.

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney Merge branch 'security/remote-address'
Security fix for `Zend\Http\PhpEnvironment\RemoteAddress`.

Fixes #5374
bb67844

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney Merge branch 'security/remote-address' into develop
Forward port #5374 (`RemoteAddress` security fix)

Conflicts:
	README.md
87b0221

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney [#5374] Fix CS issues
- trailing whitespace
cc70802

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney [#5374] Updated RemoteAddr session validator tests
- Ensured they were testing what they should, based on the changes to
  Zend\Http\PhpEnvironment\RemoteAddress
433c2de

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup'
Fix CS and testing issues introduced by security fix for #5374
efe1143

@weierophinney weierophinney added a commit that referenced this issue Oct 31, 2013

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup' into develop
Forward-ports testing and CS fixes for #5374
deffbe2

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'security/remote-address'
Security fix for `Zend\Http\PhpEnvironment\RemoteAddress`.

Fixes zendframework/zendframework#5374
b653ea9

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'security/remote-address' into develop
Forward port zendframework/zendframework#5374 (`RemoteAddress` security fix)

Conflicts:
	README.md
fa17926

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#5374] Fix CS issues
- trailing whitespace
2dd4cc5

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup'
Fix CS and testing issues introduced by security fix for zendframework/zendframework#5374
ac0446b

@weierophinney weierophinney added a commit to zendframework/zend-http that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup' into develop
Forward-ports testing and CS fixes for zendframework/zendframework#5374
f8c3502

@weierophinney weierophinney added a commit to zendframework/zend-session that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#5374] Updated RemoteAddr session validat…
…or tests

- Ensured they were testing what they should, based on the changes to
  Zend\Http\PhpEnvironment\RemoteAddress
b778a0c

@weierophinney weierophinney added a commit to zendframework/zend-session that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup'
Fix CS and testing issues introduced by security fix for zendframework/zendframework#5374
4d8e4f7

@weierophinney weierophinney added a commit to zendframework/zend-session that referenced this issue May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/5374-cleanup' into develop
Forward-ports testing and CS fixes for zendframework/zendframework#5374
5ed4ac8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment