hanover-computing/got-ssrf

SSRF doesn't work with IPv6 URLs

Kikobeats opened this issue · 11 comments

Hello,

I noted the SSRF protection is not working effectively against IPv6 URLs

For example, http://[::ffff:7f00:1]:1338/ is a valid URL; let's use got-ssrf for hitting it:

import { gotSsrf } from 'got-ssrf'
const result = await gotSsrf('http://[::ffff:7f00:1]:1338/')
console.log(result.body)

In case you want to run a local server, here is the code:

const { listen } = require('async-listen')
const { createServer } = require('http')

const html = `
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
</head>
<body>
  <h1>OH NO YOU'RE COMPROMISED</h1>
</body>
</html>`

const server = () =>
  createServer((req, res) => {
    res.setHeader('Content-Length', Buffer.byteLength(html))
    res.setHeader('Content-Type', 'text/html; charset=UTF-8')
    res.end(html)
  })

Promise.resolve().then(async () => {
  const serverIPv4url = await listen(server(), {
    host: '192.168.4.50',
    port: '1337'
  })

  const serverIPv6url = await listen(server(), {
    host: '::ffff:127.0.0.1',
    port: '1338'
  })

  console.log('> Listening at IPv4:', serverIPv4url.toString())
  console.log('> Listening at IPv6:', serverIPv6url.toString())
})

The SSRF protection is not working for a little implementation detail at this point:

https://github.com/hanover-computing/got-ssrf/blob/master/index.js#L33

The options.url.hostname there is [::ffff:7f00:1]. That is the URL hostname representation of the hostname according to whatwg URL spec:

https://url.spec.whatwg.org/#host-serializing

However, brackets there should be removed in order to pass the value against DNS lookup.

If the code is replaced by something like:

const hostname = options.url.hostname.replace('[', '').replace(']', '')
const { address } = await lookup(hostname)

Then hit the server throw the same error as IPv4 version:

RequestError: The IP of the domain is reserved!

Hello, that's a really interesting edge case. I'll have to research IPv6 a bit, but the problem seems clear to me. I'll get back to you in a bit!

Just to be clear - we want the ssrf check for http://[::ffff:7f00:1]:1338/ to pass, but it's currently failing due to trying to look up [::ffff:7f00:1], is that right?

yes

I'm trying to figure out what would be the best approach here - given that the URL scheme is complex, I'm wary of just arbitrarily "massaging" it (what untended consequences might stripping the [ and ] have for other, non-IP URLs?).

Rather, I'm thinking it might be best to forego the DNS lookup entirely if the address is a "raw" IP (using something like net.isIP. Thoughts?

Basically, the problem comes down to the fact that I am not confident that I'll be able to implement the entirety of https://url.spec.whatwg.org/#host-parsing correctly.

I think at that point is okay strip brackets using a regex; something in this way:
https://regex101.com/r/ic7HH6/1

I did some digging, and this seems to be to be the correct way of handling things (please let me know if I'm incorrect):

// Check if the hostname is an IP address - we don't need to "lookup" IP addresses!
  let IP
  const { hostname } = options.url

  if (IPv4.isIPv4(hostname)) {
    // IPv4 - we can use it "as-is"
    IP = hostname
  } else if (
    // Per https://url.spec.whatwg.org/#host-parsing,
    // if the URL starts with a [, we need to check if it ends with a ], and is an IPv6.
    hostname.startsWith('[') &&
    hostname.endsWith(']') &&
    IPv6.isIPv6(hostname.slice(1, -1))
  ) {
    // IPv6 - we need to check if it's an IPv4-mapped address: https://blog.apify.com/ipv4-mapped-ipv6-in-nodejs/
    IP = ip.process(hostname).toString()
  } else {
    // A regular hostname - we need to do a DNS lookup to get the IP address
    const { address } = await lookup(hostname)
    IP = address
  }

  // Another layer of protection against SSRF - ensure we're not hitting internal services
  // Try to match "reserved" IP ranges: https://en.wikipedia.org/wiki/Reserved_IP_addresses
  // https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html#case-2-application-can-send-requests-to-any-external-ip-address-or-domain-name
  // The function returns 'unicast' or the name of the reserved IP range, should it match any.
  // This in effect blocks all private IP Range: https://git.io/JWy3u, https://git.io/JWy3b
  if (ip.parse(IP).range() !== 'unicast')
    throw new Error('The IP of the domain is reserved!')

Basically, we break it down into IPv4 (skip), IPv6 (need to see if it's IPv4-mapped, otherwise the IPv4-mapped IPv6's ipaddr.parse().range() will show up as NOT unicast, but rather ipv4mapped!), and "regular hostname" (only at which point do we do an actual DNS lookup).

(I know the "hey the IPv4-mapped IPv6 shows up as NOT unicast" isn't part of the ticket, but I wanted to address it here regardless)

Thoughts? (If this is acceptable, I have a LOT of tests to write)

I was thinking about that too: maybe it could be worth it to take advantage of ipaddr.js and check IP type rather than lookup directly.

I found another easy way for turning 'ipv4Mapped' into real ipv4 you:

const ip = require('ipaddr.js')
ip.parse('::ffff:7f00:1').range() // => 'ipv4Mapped'
// turning 'ipv4Mapped' into 'ipv4'
ip.parse('::ffff:a9fe:a9fe').toIPv4Address().toString()

So you only have to use lookup when it is strictly necessary (hostname resolution rather than ip resolution)

Yeah, I think my final solution is going to be this (if you have concerns/objections, now would be the time to raise it):

  // Check if the hostname is an IP address - we don't need to "lookup" IP addresses!
  let IP
  const { hostname } = options.url

  if (ip.IPv4.isIPv4(hostname)) {
    IP = hostname
  } else if (
    // Per https://url.spec.whatwg.org/#host-parsing,
    // if the hostname starts with a [, we need to check if it ends with a ], and is an IPv6.
    hostname.startsWith('[') &&
    hostname.endsWith(']') &&
    ip.IPv6.isIPv6(hostname.slice(1, -1)) // strip the first and last characters - the brackets
  ) {
    IP = hostname.slice(1, -1)
  } else {
    // A regular hostname - we need to do a DNS lookup to get the IP address
    const { address } = await lookup(hostname)
    IP = address
  }

  // Another layer of protection against SSRF - ensure we're not hitting internal services.
  // Try to match "reserved" IP ranges: https://en.wikipedia.org/wiki/Reserved_IP_addresses
  // https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html#case-2-application-can-send-requests-to-any-external-ip-address-or-domain-name
  // The function returns 'unicast' or the name of the reserved IP range, should it match any.
  // This in effect blocks all private IP Range: https://git.io/JWy3u, https://git.io/JWy3b
  // We use ip.process() here to deal with potentially IPv4-mapped IPv6 addresses (which will show up as "ipv4mapped"
  // and not the whatever range the actual IPv4 address actually belongs to).
  if (ip.process(IP).range() !== 'unicast')
    throw new Error('The IP of the domain is reserved!')

(ip.process() handles that IPv4-mapping bit for us)

looks good to me!

Hello, the fix is now live on v1.3.1: https://github.com/hanover-computing/got-ssrf/releases/tag/v1.3.1, addressed on commit 5ecbc54.