HTTP redirection attack bypass DNS check

[Kikobeats]

Hello,

This library is fantastic. Thanks for that. I found an attack that is not being covered.

I setup a DNS record on my domain pointing to itself:

Nothing evil there. However, I added a redirection rule to redirect that URL into something evil:

So the library is preventing malicious attacks by checking for the hostname.

https://github.com/JaneJeon/got-ssrf/blob/master/index.js#L35

There the hostname is going to be 'local.urlint.co' being resolved into a unicast IP address.

However, if you interact with the resource, that URL is going to redirect you into the evil URL

$ curl -I -s -X GET https://local.urlint.co/dns
HTTP/2 301
date: Fri, 24 Sep 2021 14:28:30 GMT
cache-control: max-age=3600
expires: Fri, 24 Sep 2021 15:28:30 GMT
location: http://192.168.0.1

That's because we checked the DNS over the hostname but not over the hostname over the location.

Plus, most HTTP clients follow redirects by default (that's the case of got and that in fact a good expected behavior).

I wrote some code as suggestions to prevent the attack:

const { headers } = await got.head(url, { followRedirect: false })
const redirectHostname = new URL(headers.location).hostname

const { address } = await cacheableLookup.lookupAsync(url.hostname)

if (
  ip.parse(address).range() !== 'unicast' ||
  ip.parse(redirectHostname).range() !== 'unicast'
) {
  throw new Error("You're evil")
}

However, I don't like it at all, since the got.head extra network request needs to be performed. Maybe that could be handle at the DNS level?

[JaneJeon]

Jesus, that’s clever. AFAIK there’s no way to “automatically” follow redirects at the DNS level, but I’ll look into the best way to resolve this!

[JaneJeon]

Hmm, it seems like following redirects is really the safest way to go about this (if not the only way). While you could “keep things at the DNS level” and look up CNAME/ALIAS records, in your example it’s not the DNS records of the redirect that’s bad, but rather the DNS record of the redirect destination (i.e. the CNAME is legit, but the 301 on /dns is not).

So I don’t see a feasible way to keep this “within” DNS, in which case the solution is simple as you’ve pointed out: just send a HEAD request with the redirect turned on and check the DNS of the destination. In case of multiple redirects, however, if local.urlint.co/dns -> local.urlint.co/dns2 -> $evil_link, do you suppose we can just follow the redirect “all the way” or manually check that every step of the redirect is safe from SSRF?

I’m thinking a beforeRedirect hook of got might be a good fit for this.

[JaneJeon]

Yeah, ok. Running the same check on beforeRedirect should definitely take care of the class of problem you're describing here. I have the implementation and I'll push the code once I have the tests to properly cover this case.

[JaneJeon]

@Kikobeats I can confirm that the issue you pointed out is fixed now with the new version of got-ssrf (v1.2.0 and higher):

[Kikobeats]

It works like a charm 💯