CORS security: reflecting any origin header value when configured to * is dangerous

[chenjj]

When CORS policy is configured to WithOrigins("*"), asp.net CORS will actively convert it to reflect any Origin header value. This kind of behavior is dangerous and has caused many security problems in the past.

Some similar security issues:
cyu/rack-cors#126
https://nodesecurity.io/advisories/148

Some related blog posts:
https://ejj.io/misconfigured-cors/
http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html

[chenjj]

Hi, is there something I forget to do to advance this report?

Thanks,
Jianjun

[brockallen]

I think the right people need to be pinged: @blowdart (also because i know he loves the attention)

[blowdart]

We've been looking at this. So yes, in theory it can do ugly things, if you end up reflecting the origin header, and you don't encode it, and it's reflected in the body. Which is pretty hard to do unless you end up using Html.Raw. Having said that we're looking at actually validating the incoming header to at least ensure it's a valid host, in terms of character validation, and ensuring you can't do * and allow credentials. We do default to not allowing credentials, so we don't exhibit one of the problems you linked to, because it's only dangerous with allow credentials and *.

We do have folks going through the portswigger blog to see if we can make the API safer and help to avoid some of that misconfiguration.

[chenjj]

Hi Barry, thanks a lot for your reply.

Yeah, setting credentials to false by default can reduce misconfigurations, but the point I want to say here is that, converting Origin: * and Credentials: true to origin reflection is insecure and incompatible with CORS standards.

Current CORS standards(both W3C CORS and WHATWG fetch standard) have a clear definition for the wildcard *, which means any domain is allowed. But they also have another important security requirement: Origin: * and Credentials: true cannot be used at the same time, to avoid overly loose permissions. Currently all browsers follow this requirement to disallow this configuration combination.

If a framework actively converts * to reflect any origin header value, it means Origin: * and Credentials: true can be used at the same time. This behavior leads to CORS protocol's security design to be bypassed, causing many misconfiguration security problems.

Therefore, I suggest frameworks to follow the standard definition of *. When a user configures Origin: *, frameworks just directly returns Access-control-Allow-Access: *. When a user configures both Origin:* and Credentials: true , frameworks should warn users that this is a misconfiguration instead of returning any Origin header.

Thanks,
Jianjun

[blowdart]

Makes sense, we'll come back when we've poked and prodded some more.

[brockallen]

I think he has a valid point and I think it'd a change for the better. But I imagine you'll get some pushback from folks that don't care/understand the subtleties. Also, it's technically a breaking change, but maybe that's ok for security reasons (I think it is :)).

[blowdart]

Yea this might have to wait till 3.0 because it's a breaking change

[chenjj]

Thanks for your reply.

FYI, here are some more similar issues:
Yii2 framework, yiisoft/yii2#16193
Tomcat CORSFilter, CVE-2018-8014, https://bz.apache.org/bugzilla/show_bug.cgi?id=62343
Go CORS, rs/cors#55

[pranavkm]

As part of aspnet/CORS@2690a3f, for 2.2.0, CorsPolicy will

a) start responding with wildcard origin (*) if a policy supports credentials and any origin is allowed.
b) log a warning that says that the current configuration is invalid.

For 3.0, as part of #3452, we can throw an error when building the policy that prevents this configuration.

[Nness]

@pranavkm @blowdart

Isn't this a breaking change? I understand it is security change, but as it returns * instead of convert to request's origin value, it breaks client side code.

From version number, it is cors 2.2 package. If it is breaking change, it should be in 3.0 not 2.2 right? How come this got passed and deployed to 2.2 package?

Personally I can get around this, since it happened on non production ready projects. I believe this should be noted down inside release note as breaking change. Am I correct? Unless I missed, inside migration from 2.1 to 2.2 documentation, it didn't mention this breaking change.

[pranavkm]

I believe this should be noted down inside release note as breaking change.

Yup, I'll get this added to the migration notes.

[coroiu]

Well if sending credentials from any origin is dangerous, then we should at least be able to turn them off. But in the javascript SignalR client, credentials are hardcoded to true in XhrHttpClient.ts. Am I missing some sort of workaround? I don't know the origins that users will be connecting from.

Edit: I can also confirm that changing that line to false fixes the issue.

[pranavkm]

Well if sending credentials from any origin is dangerous,

Sending credentials from arbitrary origins is the source of the vulnerability. Limiting it to a well-known list of origins should work. I haven't gotten around to diving in to this further, but I don't have a recommendation for allowing credentials from arbitrary origins as yet.