Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct REMOTE_ADDR usage makes assumptions about infrastructure configuration #99

Open
chillu opened this issue Jul 6, 2021 · 0 comments

Comments

@chillu
Copy link
Member

chillu commented Jul 6, 2021

https://github.com/silverstripe/cwp-core/blob/2/src/Control/CwpBasicAuthMiddleware.php#L82

$_SERVER[REMOTE_ADDR] should not be used directly, we've got HTTPRequest->getIP() for this purpose. Specifically, the HTTPRequest IP can be optionally overwritten with proxy IPs instead (through trusted proxy IPs and X-Forwarded-For headers). This works in other parts of framework.

When this code was written, it was reasonable to make assumptions about the infrastructure configuration. In the only valid WAF proxy available on CWP (Imperva), this handling of proxy IPs happens transparently in the infrastructure. This recipe might evolve beyond CWP though (e.g. porting code over, renaming the module), at which point it'll need to deal with other infrastructure proxy layers - and use the wrong information.

This isn't a security issue as such (doesn't allow bypass of the IP allowlist), instead it'll lock out everyone when infrastructure proxies are configured differently (IP allowlist will never match).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants