Patching Go’s leaky HTTP clients

March 7, 2024

Engineering

In November 2023 we discovered an issue in the Go standard library’s net/http.Client that allowed an attacker who controls redirect targets on a server to exfiltrate authentication secrets. Soon after, we discovered a similar issue in net/http/cookiejar.Jar. The issues, collectively designated CVE-2023-45289, have now been fixed in Go 1.22.1 and Go 1.21.8, released on March 5, 2024. This blog post dives into the technical details behind those two bugs and the patch that addresses them. No published versions of Mattermost were affected by the discovered vulnerability for different reasons that are explained below.

What’s vulnerable

The issues affect Go-based HTTP clients in two slightly different configurations. If you’re using Go to make authenticated requests to an HTTP-based API, chances are that you’re at risk of being bitten by one of the bugs.

Sensitive header values leaked to 3rd parties

The first vulnerability manifests itself in code that looks like the following:

r, _ := http.NewRequest("GET", serverURL, nil)
r.Header.Set("Authorization", "secret auth header value")
http.DefaultClient.Do(r)

If the server at serverURL responds with a 3xx status code and a redirection target in the Location header, the default client follows that redirection. But a carefully crafted redirection target also gets the value of the Authorization header forwarded to it, even if the target is not on the same host as the original URL.

Cookies leaked to 3rd parties

The second vulnerability is very similar:

r, _ := http.NewRequest("GET", serverURL, nil)
http.DefaultClient.Jar, _ = cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
http.DefaultClient.Do(r)

Assuming a similarly crafted redirection as in the previous case, the client forwards any cookies set by the original host to the target host.

Full demonstrations of the issues can be found on the Go Playground here and here.

Assessing the risk

As you may have noticed above, the exploitability of the vulnerabilities is dependent on server behavior: whether or not an attacker is able to coerce the server into redirecting to an arbitrary URL. This type of server-side issue is also known as an open redirection vulnerability. Typically open redirection vulnerabilities are considered low-severity, mainly exploitable for phishing purposes. Sometimes open redirections are also intentional.

Whether intentional or not, an open redirection in an HTTP API suddenly turns into a high-severity issue when combined with a client vulnerable to CVE-2023-45289. And especially when dealing with 3rd-party APIs, it’s not always easy ruling out the possibility of open redirections hiding somewhere.

What’s more, the cookie portion of CVE-2023-45289 can also be exploited without an open redirection; as long as you use the same net/http/cookiejar.Jar instance for both cookie-authenticated requests and unauthenticated, attacker-controlled requests, an attacker will be able to get at your cookies.

Ask these questions to determine whether you’re vulnerable:

  1. Are you using Go’s net/http.Client? If not, you’re safe, but don’t forget about indirect uses, such as those in the API client libraries you import.
  2. Are any of your HTTP requests authenticated with cookies or the Authorization header? If not, you’re safe.
  3. Is any of the upstream server software you connect to controlled and maintained by you? If yes, continue with the questions below, otherwise skip forward.
    • Does the server ever redirect? Inspect the source code to confirm. If the answer is no, skip forward.
    • Are any of the redirections externally controlled? If yes, the client that connects to this server is likely vulnerable.
  4. Is any of the upstream server software you connect to controlled and maintained by a third party? If yes, for any non-trivial server you should assume the worst: the client that connects to it is likely vulnerable.
  5. If you use cookie authentication, are you using the same cookie jar to connect to multiple servers? If yes, continue with the questions below.
    • Is it possible for an attacker to control any of the URLs you connect to? If yes, the client that uses this cookie jar is likely vulnerable.

Mitigating the vulnerability

The easiest way to ensure you’re not affected by these vulnerabilities is to upgrade to Go version 1.22.1 or 1.21.8. But if you’re stuck on an older version, you may still be able to make some code changes to work around the issue.

Your options include preventing a vulnerable client from following redirections, carefully compartmentalizing the use of cookie jars, patching any open redirection issues in upstream servers, and sanitizing inputs to ensure an attacker can’t control your clients’ behavior. None of these approaches are foolproof, however, and at the very least you should understand the vulnerabilities thoroughly before attempting them.

The Authorization header part of CVE-2023-45289 only affects Go 1.21 and newer, so downgrading to 1.20 may be another option, but since the latest release already also contains other security fixes that were not backported to 1.20, that approach is not recommended.

Where it went wrong

The first part of CVE-2023-45289 was introduced in this change that touches the net/http.shouldCopyHeaderOnRedirect function. That function is called every time a client follows a redirection, determining whether headers set on the original request should be copied over to the redirected request. The logic is simple: If the header is one of a small number of headers deemed sensitive, such as Authorization, only copy it if the redirected host is the same host as the original or its subdomain. Otherwise, copy the header unconditionally.

This is what the change looks like:

func shouldCopyHeaderOnRedirect(headerKey string, initial, dest *url.URL) bool {
	switch CanonicalHeaderKey(headerKey) {
	case "Authorization", "Www-Authenticate", "Cookie", "Cookie2":
		// Permit sending auth/cookie headers from "foo.com"
		// to "sub.foo.com".

		// Note that we don't send all cookies to subdomains
		// automatically. This function is only used for
		// Cookies set explicitly on the initial outgoing
		// client request. Cookies automatically added via the
		// CookieJar mechanism continue to follow each
		// cookie's scope as set by Set-Cookie. But for
		// outgoing requests with the Cookie header set
 		// directly, we don't know their scope, so we assume
 		// it's for *.domain.com.
 
-		ihost := canonicalAddr(initial)
-		dhost := canonicalAddr(dest)
+		ihost := idnaASCIIFromURL(initial)
+		dhost := idnaASCIIFromURL(dest)
 		return isDomainOrSubdomain(dhost, ihost)
 	}
 	// All other headers are copied:
	return true
}

The only thing changed is the way hosts are canonicalized: canonicalAddr versus the new idnaASCIIFromURL. The concrete difference between the two is that the original canonicalAddr function returns the host with the port number, whereas the new function only returns the host – the intent of the change, of course, being that the logic should be relaxed to allow copying regardless of port number.

Internally, canonicalAddr uses net.JoinHostPort to include the port in the return value, and idnaAsciiFromURL, of course, does not. What’s the big deal? Let’s look at the documentation:

// JoinHostPort combines host and port into a network address of the
// form "host:port". If host contains a colon, as found in literal
// IPv6 addresses, then JoinHostPort returns "[host]:port".
//
// See func Dial for a description of the host and port parameters.

Since idnaAsciiFromURL doesn’t call net.JoinHostPort, it returns IPv6 hosts without a port number, but also without the surrounding square brackets! And this is an issue because the isDomainOrSubdomain function that does the actual matching uses suffixes to detect subdomains:

	// If sub is "foo.example.com" and parent is "example.com",
	// that means sub must end in "."+parent.
	// Do it without allocating.
	if !strings.HasSuffix(sub, parent) {
		return false
	}

As long as there are square brackets around IPv6 addresses, they cannot be confused with domain names, because an IPv6 address will always end with the ] character, and no domain name does. But when you remove the square brackets, everything falls apart:

isDomainOrSubdomain(`2001:DB8::1%.example.com`, `example.com`) // true

The %.example.com suffix in 2001:DB8::1%.example.com is a zone identifier, and a perfectly valid one at that. But the isDomainOrSubdomain function, based on nothing but suffixes, gets confused.

The second part of CVE-2023-45289 is almost identical in its root cause: the net/http/cookiejar.hasDotSuffix method is used to match cookies to hosts, and as the name suggests, it performs simple suffix-based matching:

// hasDotSuffix reports whether s ends in "."+suffix.
func hasDotSuffix(s, suffix string) bool {
	return len(s) > len(suffix) && s[len(s)-len(suffix)-1] == '.' && s[len(s)-len(suffix):] == suffix
}

The cookie jar implementation never used net.JoinHostPort, so older versions of Go are also vulnerable.

How it was fixed

The fix to both vulnerabilities can be found in Go’s 1.22 branch here and in 1.21 here. The first part of the fix, in net/http/client.go, looks like this:

func isDomainOrSubdomain(sub, parent string) bool {
	if sub == parent {
		return true
	}
+	// If sub contains a :, it's probably an IPv6 address (and is definitely not a hostname).
+	// Don't check the suffix in this case, to avoid matching the contents of a IPv6 zone.
+	// For example, "::1%.www.example.com" is not a subdomain of "www.example.com".
+	if strings.ContainsAny(sub, ":%") {
+		return false
+	}
	// If sub is "foo.example.com" and parent is "example.com",
	// that means sub must end in "."+parent.
	// Do it without allocating.
	if !strings.HasSuffix(sub, parent) {
		return false
	}
	return sub[len(sub)-len(parent)-1] == '.'
}

The check and the comment above it are fairly self-explanatory. The fix is simple and effective.

The second part uses an identical check but places it in a separate function used for detecting IP addresses and handling them in a separate code path:

// isIP reports whether host is an IP address.
func isIP(host string) bool {
+	if strings.ContainsAny(host, ":%") {
+		// Probable IPv6 address.
+		// Hostnames can't contain : or %, so this is definitely not a valid host.
+		// Treating it as an IP is the more conservative option, and avoids the risk
+		// of interpeting ::1%.www.example.com as a subtomain of www.example.com.
+		return true
+	}
	return net.ParseIP(host) != nil
}

This same function was used before, but it would fail with IP addresses that include a zone identifier: net.ParseIP expects an IP address without a zone, and returns nil if it encounters a non-empty zone identifier.

Mattermost products not affected

Mattermost products use Go’s HTTP clients heavily, from fetching OpenGraph metadata to implementing OAuth and integrating with external services like GitHub and Jira. No Mattermost products were affected by these vulnerabilities, however, for two main reasons:

  1. Mattermost policy for the main product is to stay on the second-to-latest major release of Go at all times. Because of this, when we discovered the issue, we still hadn’t upgraded to Go 1.21; Go 1.22.0 hadn’t been released yet. And when 1.22.0 did come out, we made the decision to hold off on upgrading until this issue had been fixed.
  2. Mattermost’s Go HTTP clients do not use net/http/cookiejar, so the second part of the issue was never relevant to us.

Disclosure timeline

Nov 23, 2023Mattermost discovers vulnerability in net/http.Client and reports it to the Go security team
Nov 28, 2023Go security team acknowledges report
Nov 30, 2023Mattermost discovers additional vulnerability in net/http/cookiejar.Jar and reports it to the Go security team
Jan 16, 2024Mattermost asks the Go security team for a status update
Jan 16, 2024Go security team confirms patches are ready, scheduled for release on the 1.22 release date, Feb 6.
Feb 2, 2024Go security team notifies Mattermost of delays, patches will be released on March 5 instead.
Feb 29, 2024Go 1.22.1 and Go 1.21.8 pre-announcement on mailing lists
Mar 5, 2024Patches released

Read more about:

Go security
mm

Juho Nurminen is a Product Security Engineer at Mattermost. Prior to joining the company, he worked as a security specialist in the consulting sector and as an R&D engineer developing an open-source application framework.

Read More Engineering Articles