cactus / go-camo

A secure image proxy server
MIT License
255 stars 48 forks source link

Ignore port when filtering IPs and Hostnames #28

Closed patrobinson closed 6 years ago

patrobinson commented 6 years ago

Description

The protection against proxying requests to local IPs can be bypassed by adding a port to the hostname (i.e. http://localhost:80) This can result in a Server Side Request Forgery (SSRF) attack, allowing an attacker that controls the URL generated to impersonate the server in making requests. This is fixed by using the Hostname() function from net/url, which only ever returns the hostname without the port. Please explain the changes you made here.

Checklist

dropwhile commented 6 years ago

PR looks good, but for some reason the tests are not running properly (they appear to be hanging). Looking into it...

patrobinson commented 6 years ago

@cactus it looks like TestTimeout was relying on this behaviour to work

dropwhile commented 6 years ago

hmm. Yeah, the local server ephemeral port would now match against the blocklist. Interesting! I am surprised that the timeout isn't firing, or that the test server isn't return a 404 though...

dropwhile commented 6 years ago

Ah. Figured it out. The testServer wasn't being hit (obviously), and there was a wait on a channel in the test, that blocked indefinitely. Just a side effect of this code change.