seanmonstar
Default RedirectPolicy is unsafe in server-side setting
Default redirect policy for reqwest is RedirectPolicy::limited(10), which is unsafe in server-side setting due to possibility of SSRF.
The issue we are having is that we need to set it to RedirectPolicy::none() every time we use reqwest client, which is something that is really easy to miss.
Not sure what to do here as changing it to RedirectPolicy::none() would be a breaking behavior (also, not sure if everyone would make the same choice between convenience vs safety).
Would it make sense to have "global" default in an atomic of some sort, so we can disable it globally, like reqwest::global_redirect_policy(RedirectPolicy::none())?
Sorry, what's unsafe about following redirects? I followed the link, but I don't see anything that redirects cause.
Here's an example vulnerability caused by redirects: https://medium.com/@rootxharsh_90844/vimeo-ssrf-with-code-execution-potential-68c774ba7c1e
The TL;DR is that a server-side application can validate a URL before making an outbound HTTP request (e.g., to a webhook server) only to have that webhook server turn around and redirect the client to some internal URL such as a local AWS metadata endpoint (e.g., http://169.254.169.254/latest/meta-data/iam/security-credentials/s3access)
See https://github.com/ramosbugs/oauth2-rs/commit/ec8f921032ff9ba8d827172f6ddcdfc692421e8e for a specific fix when using reqwest as the client.
Thanks for that, I understand now. So curl doesn't automatically follow redirects, but Golang, python-requests, and request.js all do. They have similar issues?
If they follow redirects by default, then yup! SSRF has been growing in popularity in recent years, but unfortunately changing the default redirect behavior is a breaking change, so it's unsurprising that these older libraries you mentioned wouldn't want to do this.
We just ran into this as well during a code audit. In our case, we are resolving the issue by setting reqwest's redirect policy to none(), and then implementing our own redirect logic that does DNS lookups and verifies that the provided URLs are publicly routed. We would like to implement that as a redirect policy, but it would require blocking inside a non-async function inside an async function in order to perform the DNS lookups.
One possible solution in our case would be a Policy::custom_future option that takes an FnMut(Attempt) -> Future<Output=Action>
