1672528 - (CVE-2020-26961) TRR (DoH) RFC1918 addresses protection bypass using IPv4-mapped address

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect, P1)

Description

[Gabriel Corona]

Summary

When using Firefox with Trusted Recursive Resolver (TRR), private IP addresses are
rejected by default (network.trr.allow-rfc1918=false). This protection can prevent
browser-based attacks of machines located on the LAN such as DNS-rebinding attacks.
However, this protection can be bypassed by using a IPv4-mapped IPV6 address
(eg. ::ffff:192.168.1.254).

This has been tested with Firefox Nightly 84.0a1 (2020-10-21) (64 bits) on Debian testing.

Example

Let's create some domain names records:

$ dig @1.1.1.1 +short A wat4.urdhr.fr
::ffff:192.168.1.254
$ dig @1.1.1.1 +short AAAA wat6.urdhr.fr
::ffff:192.168.1.254

Let's check the are actually resolved by the DoH resolver
we are using:

import dns.query
import dns.message
resolver = "https://cloudflare-dns.com/dns-query"
dns.query.https(dns.message.make_query("wat4.urdhr.fr", "A"), resolver).answer
# => [<DNS wat4.urdhr.fr. IN A RRset: [<192.168.1.254>]>]
dns.query.https(dns.message.make_query("wat6.urdhr.fr", "AAAA"), resolver).answer
# => [<DNS wat6.urdhr.fr. IN AAAA RRset: [<::ffff:192.168.1.254>]>]

When using TRR and network.trr.allow-rfc1918=false,

• http://wat4.urdhr.fr is not reachable in TRR
  mode because the private IP address is rejected by Firefox;

• http://wat6.urdhr.fr however, is reachable
  (assuming a web server is running on this IP address and port).

As far as I understand, the process is as follow:

1. the name is resolved to IPv6 ffff:192.168.1.254;
2. this IPv6 address is not considered a private IP address by NetAddr::IsIPAddrLocal();
3. this IPv6 address generates a AF_INET6 address in DOHresp::Add();
4. this AF_INET6 is passed to kernel;
5. kernel connects to 192.168.1.256.

Code snippets

NetAddr derivation from DNS bytes:

nsresult DOHresp::Add(uint32_t TTL, unsigned char* dns, unsigned int index,
                      uint16_t len, bool aLocalAllowed) {
  NetAddr addr;
  if (4 == len) {
    // IPv4
    addr.inet.family = AF_INET;
    addr.inet.port = 0;  // unknown
    addr.inet.ip = ntohl(get32bit(dns, index));
  } else if (16 == len) {
    // IPv6
    addr.inet6.family = AF_INET6;
    addr.inet6.port = 0;      // unknown
    addr.inet6.flowinfo = 0;  // unknown
    addr.inet6.scope_id = 0;  // unknown
    for (int i = 0; i < 16; i++, index++) {
      addr.inet6.ip.u8[i] = dns[index];
    }
  } else {
    return NS_ERROR_UNEXPECTED;
  }

  if (addr.IsIPAddrLocal() && !aLocalAllowed) {

    return NS_ERROR_FAILURE;
  }

  // While the DNS packet might return individual TTLs for each address,
  // we can only return one value in the AddrInfo class so pick the
  // lowest number.
  if (mTtl < TTL) {
    mTtl = TTL;
  }

  if (LOG_ENABLED()) {
    char buf[128];
    addr.ToStringBuffer(buf, sizeof(buf));
    LOG(("DOHresp:Add %s\n", buf));
  }
  mAddresses.AppendElement(addr);
  return NS_OK;
}

Checking if IP address is private:

bool NetAddr::IsIPAddrLocal() const {
  const NetAddr* addr = this;

  // IPv4 RFC1918 and Link Local Addresses.
  if (addr->raw.family == AF_INET) {
    uint32_t addr32 = ntohl(addr->inet.ip);
    if (addr32 >> 24 == 0x0A ||    // 10/8 prefix (RFC 1918).
        addr32 >> 20 == 0xAC1 ||   // 172.16/12 prefix (RFC 1918).
        addr32 >> 16 == 0xC0A8 ||  // 192.168/16 prefix (RFC 1918).
        addr32 >> 16 == 0xA9FE) {  // 169.254/16 prefix (Link Local).
      return true;
    }
  }
  // IPv6 Unique and Link Local Addresses.
  if (addr->raw.family == AF_INET6) {
    uint16_t addr16 = ntohs(addr->inet6.ip.u16[0]);
    if (addr16 >> 9 == 0xfc >> 1 ||    // fc00::/7 Unique Local Address.
        addr16 >> 6 == 0xfe80 >> 6) {  // fe80::/10 Link Local Address.
      return true;
    }
  }
  // Not an IPv4/6 local address.
  return false;
}

Tests

These tests should be able to check the expected behavior (in test_trr.js):

// verify RFC1918 IPv4-mapped address from the server is rejected
add_task(async function test7() {
  dns.clearCache(true);
  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
  Services.prefs.setCharPref(
    "network.trr.uri",
    `https://foo.example.com:${h2Port}/doh?responseIP=::ffff.192.168.0.1`
  );
  let [, , inStatus] = await new DNSListener(
    "rfc1918.example.com",
    undefined,
    false
  );
  Assert.ok(
    !Components.isSuccessCode(inStatus),
    `${inStatus} should be an error code`
  );
});

// verify RFC1918 IPv4-mapped address from the server is fine when told so
add_task(async function test8() {
  dns.clearCache(true);
  Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
  Services.prefs.setCharPref(
    "network.trr.uri",
    `https://foo.example.com:${h2Port}/doh?responseIP=::ffff.192.168.0.1`
  );
  Services.prefs.setBoolPref("network.trr.allow-rfc1918", true);
  await new DNSListener("rfc1918.example.com", "192.168.0.1");
});

Additional Considerations

It would probably make sense to be able to reject the following networks
as well:

• 0.0.0.0/24
• 127.0.0.0/8
• ::1/128

These can be used for DNS-rebinding attacks on the local machine.

Comment 1

[Daniel Veditz [:dveditz]]

What TRR mode do you use? I'm pretty sure by default we simply bypass DoH for RFC1918 addresses and resolve them locally. It's not a protection, it's because the remote TRR isn't going to know anything about your local network. However, if you've disabled fallback then maybe you get some accidental protection.

We have other proposals to protect against local connections: see "CORS for RFC1918" proposals.

Dragana: is the first bit correct? What is the purpose of the network.trr.allow-rfc1918 pref

Comment 2

[Gabriel Corona]

What TRR mode do you use?

I'm using network.trr.mode=2.

I'm pretty sure by default we simply bypass DoH for RFC1918 addresses

I'm not sure I understand this. I think you are talking about the fact that when I'm visiting http://192.168.0.1, DoH won't be used to resolve 192.168.0.1 into an IP address.

This is not what network.trr.allow-rfc1918 is about.

What is the purpose of the network.trr.allow-rfc1918 pref

As far as I understand, network.trr.allow-rfc1918=true tells that when I'm resolving www.example.com into an IP address (for example because I'm visiting http://www.example.com), Firefox will ignore any private IP address obtained over DoH, i.e. :

1. Firefox want to connect to http://www.example.com
2. Firefox tries to resolve www.example.com using DoH
3. DoH resolver answers with 198.168.0.1
4. address is ignored by Firefox (when network.trr.allow-rfc1918=true)

I suppose this feature is implemented as protection against DNS-based attacks. In particular, this block many DNS-rebinding attacks against vulnerable services on the LAN.

Some resolvers have support for blocking private IP addresses [1] this . For example, the resolvers of my ISP implement this feature [2] (I would argue that this practice is questionable). By enabling TRR on Firefox, the local/ISP resolvers are bypassed. So, enabling TRR might end up opening DNS-rebinding attacks

Note: network.trr.allow-rfc1918 controls aLocalAllowed in DOHresp::Add() which is called when processing a DoH response.

We have other proposals to protect against local connections: see "CORS for RFC1918" proposals.

Good to know. I was not aware of this! I'll look at this.

[1] https://docs.netgate.com/pfsense/en/latest/services/dns/rebinding.html#dns-resolver-unbound

[2] https://www.gabriel.urdhr.fr/2020/09/23/dns-rebinding-freebox/#fn:private

Comment 3

[Valentin Gosu [:valentin] (he/him)]

(In reply to Gabriel Corona from comment #0)

Summary

When using Firefox with Trusted Recursive Resolver (TRR), private IP addresses are
rejected by default (network.trr.allow-rfc1918=false). This protection can prevent
browser-based attacks of machines located on the LAN such as DNS-rebinding attacks.
However, this protection can be bypassed by using a IPv4-mapped IPV6 address
(eg. ::ffff:192.168.1.254).

1. the name is resolved to IPv6 ffff:192.168.1.254;

This is a good catch. Thanks for the report.

Additional Considerations

It would probably make sense to be able to reject the following networks
as well:

• 0.0.0.0/24
• 127.0.0.0/8
• ::1/128

These can be used for DNS-rebinding attacks on the local machine.

Normally these shouldn't be a problem - the loaded page can always try to access http://127.0.0.1:[port] regardless of the domain. We might add a pref for these anyways, but I don't think we need to change the default unless you can think of a way to exploit this.

Comment 4

[Gabriel Corona]

the loaded page can always try to access http://127.0.0.1:[port] regardless of the domain.

Yes but the same thing can be said of http://192.168.0.1:[port].

For both cases (private and localhost IP address) , the problem (in the case of DNS-rebinding attacks) is that these endpoints are accessed while being considered an other origin (eg. www.example.com).

unless you can think of a way to exploit this.

DNS-rebinding attacks can be conducted against 127.0.0.1 (or 0.0.0.0, ::1) juste as well:

1. user visits http://www.example.com:1234
2. Browser looks up www.example.com and gets 10.0.0.1
3. Browse connects to 10.0.0.1:1234 and receives malicious JavaScript code
4. Malicious JS code calls fetch("/some_resource")
5. Browser looks up www.example.com again and gets 127.0.0.1
6. Browser makes GET http://www.example.com:1234/some_resource against 127.0.0.1:1234
7. Browser sees the request as being same-origin and exposes response to JS code
8. Browser exfiltrates response to other.example.com

While this example is about a GET, we could make other kind of requests as well.

Comment 5

[Gabriel Corona]

For reference, I found this after reading an advisory about CVE-2020-26887.

Comment 6

[Gabriel Corona]

Woops, in steps (2) and (3), the IP address was meant to be an example one (eg. 192.0.2.1).

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1672528 - Check IPv4-mapped IPv6 addresses for being local r=dragana

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1672528 - Test

Depends on D95414

Comment 9

[Valentin Gosu [:valentin] (he/him)]

(In reply to Gabriel Corona from comment #0)

Additional Considerations

It would probably make sense to be able to reject the following networks
as well:

• 0.0.0.0/24
• 127.0.0.0/8
• ::1/128

Do you think we should do something when a resolver returns one of these IPs?
I think right now we don't have any specific handling for 0.0.0.0, even though it could be used to connect to localhost.

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 9184932 [details]
Bug 1672528 - Check IPv4-mapped IPv6 addresses for being local r=dragana

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unclear how easy it would be to actually exploit this.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Easy to port to esr78
• How likely is this patch to cause regressions; how much testing does it need?: Low risk of regressions. This patch ensures that we apply the same checks to IPv4 mapped IPv6 addresses.

Comment 11

[Gabriel Corona]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #9)

Do you think we should do something when a resolver returns one of these IPs?
I think right now we don't have any specific handling for 0.0.0.0, even though it could be used to connect to localhost.

It would probably make sense to consider all these IP as "private".

See a related ticket I opened about cors-rfc1918.

Comment 12

[Tom Ritter [:tjr]]

Comment on attachment 9184932 [details]
Bug 1672528 - Check IPv4-mapped IPv6 addresses for being local r=dragana

This; and the test, are okay to land and request uplift. Reproducing the issue is trivial from the patch, so there is no need to delay the test and incur the cognitive overhead of remembering to land it later.

Comment 13

[Tom Ritter [:tjr]]

Comment on attachment 9184933 [details]
Bug 1672528 - Test

Also, please request uplift if needed.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/179e399ac08119ef3da61766c73f265679a6cf51
https://hg.mozilla.org/integration/autoland/rev/b18b9e52050cb671b57c011daa4a173fe46aec5a

Please nominate this for Beta/ESR78 approval ASAP. It grafts cleanly as-landed.

Comment 15

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 9184932 [details]
Bug 1672528 - Check IPv4-mapped IPv6 addresses for being local r=dragana

Beta/Release Uplift Approval Request

• User impact if declined: Potential bypass of local address mitigations
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only ensures that we apply the same checks to IPv4 mapped IPv6 addresses.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Security problem is quite obvious.
• User impact if declined: Potential bypass of local address mitigations
• Fix Landed on Version: 84
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only ensures that we apply the same checks to IPv4 mapped IPv6 addresses.
• String or UUID changes made by this patch:

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/179e399ac08119ef3da61766c73f265679a6cf51
https://hg.mozilla.org/integration/autoland/rev/b18b9e52050cb671b57c011daa4a173fe46aec5a
https://hg.mozilla.org/mozilla-central/rev/179e399ac081
https://hg.mozilla.org/mozilla-central/rev/b18b9e52050c

Comment 17

[Frederik Braun [:freddy]]

(ckerschb asked me to steal this NI)

(In reply to Valentin Gosu [:valentin] (he/him) from comment #9)

(In reply to Gabriel Corona from comment #0)

Additional Considerations

It would probably make sense to be able to reject the following networks
as well:

• 0.0.0.0/24
• 127.0.0.0/8
• ::1/128

Do you think we should do something when a resolver returns one of these IPs?
I think right now we don't have any specific handling for 0.0.0.0, even though it could be used to connect to localhost.

I think we ought to try in a separate mechanism that's easy to revert (e.g., with a pref). There's a risk this will break stuff for our users when we block things resolving to localhost. I'm not too concerned about 0.0.0.0 though.

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 9184932 [details]
Bug 1672528 - Check IPv4-mapped IPv6 addresses for being local r=dragana

approved for 83.0b9 and 78.5esr

Comment 19

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ad0f94856a65
https://hg.mozilla.org/releases/mozilla-beta/rev/220257bd34e8

Comment 20

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/024502f9880eaedf9a17edb3b5c0976cf32d44f5
https://hg.mozilla.org/releases/mozilla-esr78/rev/969531806d0fbb893103c59d7beb22213ea080ab

(had to resolve a textual conflict around bug 1663657, but that looked trivial enough)

Comment 21

[Tom Ritter [:tjr]]

Attachment: advisory.txt