Server Side Request Forgery (SSRF)

[freddyb]

As reported with wallabag, graby is vulnerable to SSRF. This means one can bypass restrictions for resources only accessible to localhost like http://127.0.0.1/server-status,

[j0k3r]

I've read few articles about how to try to fix that, like this one: http://www.riyazwalikar.com/2012/11/cross-site-port-attacks-xspa-part-3.html

1. Error handling and message:
   Try to display common error message to avoid data leak (I think it's already done)
2. Restrict connectivity to HTTP based ports:
   Should be restrict url to target 80 & 443 and don't care about others ?
3. Blacklist IP addresses:
   Like 127.* and localhost
4. Disable unwanted protocols
   Keep http(s) is enough I think

[freddyb]

Note that the IP address blacklist must happen after resolving domain names. DNS checking can be tricky since you must assume that the attacker controls the DNS and can return a different IP when you check and when you use it again for doing the request (TOCTOU).

The blacklist should contain much more than just 127.* and localhost. Consider other local addresses (link-local, LAN, …) and IPv6.

Happy to look at your patch and attempt to bypass, if that helps.

[j0k3r]

About your first point, should gethostbynamel be enough?

I think I can grab some good stuff from https://github.com/fin1te/safecurl

Like these local addresses:

0.0.0.0/8
10.0.0.0/8
100.64.0.0/10
127.0.0.0/8
169.254.0.0/16
172.16.0.0/12
192.0.0.0/29
192.0.2.0/24
192.88.99.0/24
192.168.0.0/16
198.18.0.0/15
198.51.100.0/24
203.0.113.0/24
224.0.0.0/4
240.0.0.0/4

[freddyb]

Maybe you can just use safecurl completely instead of curl? It has a good security track record!

[j0k3r]

Yeah, that could be a good idea but I'm not using cURL directly I'm using Guzzle. I need to check how can I use them together.

[freddyb]

Security is never easy ;-)
Maybe you could look into how much work it is to create a guzzle+safecurl fork?

[j0k3r]

I don't think I'll need to created a fork, maybe a simple PR to SafeCurl should be enough since Guzzle v6 add ability to define a different handler: https://github.com/guzzle/guzzle/blob/master/src/Handler/CurlHandler.php

I just need to create a custom one for SafeCurl.
I'll see :)