CORS: unconditionally reflecting Origin in Access-Control-Allow-Origin is insecure

[jub0bs]

Issue Description

Echo's CORS middleware actively bypasses the so-called wildcard exception: if developers configure their CORS middleware to allow credentials and specify the wildcard as an allowed origin, the resulting middleware unconditionally reflects the value of the request's Origin header in the Access-Control-Allow-Origin response header.

This is insecure insofar as it exposes users to cross-origin attacks that can be mounted from any origin.

For information, a similar issue was reported to (and subsequently fixed by) other Web frameworks/libraries:

• Go Chi
• rs/cors

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

Perhaps the following:

curl -sD - -o /dev/null \
  -H "Origin: https://attacker.org" \
  localhost:8081/hello

HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: *
-snip-

Ideally, though, the resulting middleware should not be built at all, since it is dysfunctional. More about this in my latest blog post.

Actual behaviour

curl -sD - -o /dev/null \
  -H "Origin: https://attacker.org" \
  localhost:8081/hello

HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: https://attacker.org
-snip-

Note the unconditional reflection of the request's Origin header value (https://attacker.org) in the Access-Control-Allow-Origin response header.

Steps to reproduce

1. Run mkdir wildcardcraziness && cd $_.
2. Save the program below to main.go.
3. 2. Run go mod init whatever && go mod tidy.
4. Run go run main.go.
5. Run curl -sD - -o /dev/null -H "Origin: https://attacker.org" localhost:8081/hello.

Working code to debug

package main

import (
  "net/http"

  "github.com/labstack/echo/v4"
  "github.com/labstack/echo/v4/middleware"
)

func main() {
  e := echo.New()
  e.Use(middleware.CORSWithConfig(middleware.CORSConfig{
    AllowOrigins:     []string{"*"},
    AllowCredentials: true,
  }))
  e.GET("/hello", hello)
  e.Logger.Fatal(e.Start(":8081"))
}

func hello(c echo.Context) error {
  return c.String(http.StatusOK, "Hello, World!")
}

Version/commit

v4.10.0

[aldas]

I do not think this is a problem. It is a opinion on responsibilities of library vs developer and where lines should be drawn.

1. AllowCredentials is by default false
2. AllowCredentials has documented warning/explanation why you should not set it true.

echo/middleware/cors.go

Lines 75 to 80 in 6b09f3f

	// Security: avoid using `AllowCredentials = true` with `AllowOrigins = *`.
	// See "Exploiting CORS misconfigurations for Bitcoins and bounties",
	// https://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
	//
	// See also: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials
	AllowCredentials bool `yaml:"allow_credentials"`

Configuration docs have itself links to resources related to CORS security:

echo/middleware/cors.go

Lines 113 to 121 in 6b09f3f

	// Security: Poorly configured CORS can compromise security because it allows
	// relaxation of the browser's Same-Origin policy. See [Exploiting CORS
	// misconfigurations for Bitcoins and bounties] and [Portswigger: Cross-origin
	// resource sharing (CORS)] for more details.
	//
	// [MDN: Cross-Origin Resource Sharing (CORS)]: https://developer.mozilla.org/en/docs/Web/HTTP/Access_control_CORS
	// [Exploiting CORS misconfigurations for Bitcoins and bounties]: https://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
	// [Portswigger: Cross-origin resource sharing (CORS)]: https://portswigger.net/web-security/cors
	func CORS() echo.MiddlewareFunc {

Ideally, though, the resulting middleware should not be built at all, since it is dysfunctional. More about this in my latest blog post.

Setting AllowCredentials to true is developer conscious decision. cors middleware does not get magically added to their application.

Hypothetical question: How different is this from these examples and when should be intervene?

	e.Use(middleware.JWTWithConfig(middleware.JWTConfig{
		SigningKey:    []byte("test"), // (hardcoded) secret that is easy to brute-force
	}))

	e.Use(middleware.JWTWithConfig(middleware.JWTConfig{
		SigningKey:    jwt.UnsafeAllowNoneSignatureType,
		SigningMethod: "none", // using no signing method at all
	}))

	e.Use(middleware.BasicAuth(func(user string, password string, c echo.Context) (bool, error) {
		return user == "test" && password == "test", nil
	}))

I think if we want to define our role in world - it is to educate community by improving documentation and have reasonably safe defaults. I do not think our role is lock developers into cradle because we assume they are toddlers.

[aldas]

We probably could come to somewhere in the middle - make shooting your foot harder and still allow maybe no so bright things, by adding UnsafeAllowWildcardOrigin configuration flag.

This thing will come up in dev/test etc environments and people just start adding config.AllowOrigins = []string{"mydomain.com", "localhost"} etc if they are not doing something like that already to get past browser blocking their requests. Although "localhost" is smaller scope than * it is still potentially problematic if we look world through monochrome security glasses.
Also vhosts could be the place where * could be used - where you could go the lazy route and instead of maintaining your list of domains, just take the risk and allow all.

If you have turned it off - should respect your unsafe choice.

p.s. https://github.com/labstack/echox/blob/master/website/content/middleware/cors.md should be improved

[aldas]

@vishr and @lammel

[jub0bs]

@aldas The documentation does warn against using AllowCredentials: true and AllowOrigins: []string{"*"} but nowhere (unless I'm missing something) does it explain the resulting middleware behaviour. Someone intimately familiar with CORS and the wildcard exception would be very surprised to see the origin reflected unconditionally in ACAO (as opposed to just having ACAO: *, for example). By bypassing the wildcard exception that way, Echo's CORS middleware is being too helpful for its users' own good.

I do not think our role is lock developers into cradle because we assume they are toddlers.

I wouldn't necessarily put it that way (in all seriousness), but I disagree. CORS consists in relaxing the SOP's restrictions; as such, it is security-critical. A good library (esp. one that deactivates existing browser defences) should not only be easy to use, but also hard to misuse. At the moment, misusing Echo's CORS middleware and completely voiding the SOP is deceptively easy.

We probably could come to somewhere in the middle - make shooting your foot harder and still allow maybe no so bright things, by adding UnsafeAllowWildcardOrigin configuration flag.

I do like the idea of forcing users to be deliberate about opting for an insecure configuration. That's the general approach I've followed for my own CORS middleware library. It goes one step further, though, by preventing (at compile time) users from allowing all origins with credentials.

[aldas]

That reflected unconditionally is undocumented/poorly documented "feature with potentially negative side-effects" :) which we need to definitely explain better. At the moment we have these security related resources only mentioned in .go files but we need to add security block to echo.labstack.com site.

Ability to choose insecure configuration is definitely needed. I have myself experienced cases: building Angular SPAs (proxying everthing though local nodejs proxy to live reloaded assets and go API somewhere) or creating ssh tunnels to tests servers at work etc. These cases I would not want to disable CORS entirely, just enough to be able to develop. Security is not all black-and-white and there are plenty of nuances where there are multitude of shades/colors. If you make it too draconian - people just opt out of it.

Anyway - additional flag to enable use-case "wildcard + allow-Credentials" with scary name should be reasonable compromise.

Implementing CORS library from the start with very opinionated way - is an option. Like choosing Mac OS vs some Linux distro. Echo middlewares are almost always very configurable. This is a genie/jinn we can not bottle anymore. You can code incredibly stupid things if you decided to do so (these JWT examples above). With freedom comes (your) personal responsibility. Or you can choose premium experience (Mac OS like) with more opinionated libraries.

[aldas]

closing, #2405 added extra flag.

Thanks @jub0bs for reporting this.