corydolphin / flask-cors

Cross Origin Resource Sharing ( CORS ) support for Flask
https://flask-cors.corydolphin.com/
MIT License
867 stars 140 forks source link

enforcing same origin policy with flask-cors #320

Open Tingweiftw opened 2 years ago

Tingweiftw commented 2 years ago

While this library is used to enable cors, is there an option to disable the default wildcard value in CORS_ORIGIN without explictly defining trusted domains in a string/list so it defaults back to same origin policy?

derek-adair commented 1 year ago

This is something that would make a great pull request ;)

Helveg commented 1 year ago

I came here to open just the same exact issue. Many, let's say 5 nines of all developers, see CORS as a frustration and just disable it, but it is an important measure to limit the severity of XSS attacks. Flask-CORS' current defaults contribute to this nonchalance by by default completely disabling CORS. The documentation should be restructured so that the index page explains how to actually safely allow access from a whitelist, and only later on, and with a big fat warning, introduce the '*' as an option!

It's bad enough to have a security vulnerability, it's worse that anyone can exploit it from the victim's machine merely by browsing to a domain the attacker controls!

derek-adair commented 1 year ago

This is a very good point. I'd be willing to guess a LOT of people here are using this project to bypass CORS altogether. It's a much more secure default. An option to disable same origin restriction is probably the safer decision here.

Helveg commented 1 year ago

To get this Same Origin behaviour, I should grab the host and protocol as the default value for origins, right? I can PR that and propose a draft for the doc index.

Since this could be seen both as a security vulnerability fix, and as a breaking change for all these people who didn't actually secure their app, do we introduce it as a patch version bump, or major version? 😂

derek-adair commented 1 year ago

this would be a major version bump a la breaking changes.

As for how this should be done I've not had a chance to think on that and would be something the maintainer would need to weigh in on.

corydolphin commented 1 year ago

Hey folks,

Loosely held opinions open to change.

First, everything above is probably accurate.

This package has a simple philosophy: when you want to enable CORS, you wish to enable it for all use cases on a domain. This means no mucking around with different allowed headers, methods, etc.

By default, submission of cookies across domains is disabled due to the security implications. Please see the documentation for how to enable credential'ed requests, and please make sure you add some sort of CSRF protection before doing so!

The main security attack vector I'm familiar with with CORS is via CSRF, which is why cookies are disabled by default. I'm open to changing this such that the default is to ask for an origin list, with special handling of "*".

Honestly, this library could use a major version bump to land a number of improvements to the API which weren't thought of in the beginning, in addition, it may be useful to introduce MyPy support and break python version support compatibility.

@derek-adair Is that something you would be interested in championing?

Helveg commented 1 year ago

The philosophy is sound, it leads to simple effortless setup of CORS. In the context of security this ease-of-use could lead to carelessness, and as the most popular package for CORS for Flask you shape the landscape quite a bit. As probably mentioned before in this thread, CORS is encountered more as an annoyance than as a security concern. That's why I'd argue the philosophy shouldn't centre 100% around ease-of-use, but also incorporate responsibility around the defaults you promote. The proper default behaviour, which will overlap with most if not all use-cases of people unfamiliar with CORS, that will blindly rely on the defaults, is that you want your app, not any app, to communicate with your server, which corresponds to the Same Origin policy.

Both CSRF and XSS attacks are enabled by lack of CORS. Without proper CORS users of your domain are vulnerable to pretty devastating "one-click attacks", where just by following a link they've received that leads to a perfectly valid domain, stealth attacks can be performed on other domains and even local services/containers running on localhost:

I came here because a developer of a scientific software decided to use Flask for a sandbox, which executes arbitrary code, and they in their browser got slapped with the CORS error, and used these default settings, turning a trusted piece of scientific software into an XSS trojan horse.

You'll also see that on any/all cloud providers, identity providers and OAuth2, the allowed CORS origins are important security settings, that they do not default to "". Asking the user to consider the question "from where will my users initiate actions?" seems like a reasonable ask for a CORS package! 😄 If they want to answer "from anywhere", it'll only cost them 1 character to reply 👍

derek-adair commented 1 year ago

@derek-adair Is that something you would be interested in championing?

...as the most popular package for CORS for Flask you shape the landscape quite a bit..

This gives me anxiety. I'll consider it but probably not. I dont think I have the right mindset for such an important project. I may however know someone who would have a LOT to add to this project that is a huge security nerd.

derek-adair commented 1 year ago

I came here because a developer of a scientific software decided to use Flask for a sandbox, which executes arbitrary code, and they in their browser got slapped with the CORS error, and used these default settings, turning their local sandbox into an XSS trojan horse.

Yaaaa. This is exactly the kind of thing some more tight defaults will prevent. Giving a novice dev the ability to bypass DECADES of progress in browsers locking down/preventing/warning devs and users about CORS is not ideal.

n1ngu commented 5 months ago

Fully agree that default should not be to whitelist '*' and the package deserves a major bump. The issue importance and severity has already been clearly stated.

Yet, AFAIU, same-origin policy can be enforced by... not using flask-cors nor anything at all?

Helveg commented 5 months ago

So what needs to be done to pull the trigger on this @corydolphin ? I have time to PR it.

n1ngu commented 5 months ago

AFAIU it boils down to simply replacing the "*" value for and empty list in flask_cors.core.DEFAULT_OPTIONS["origins"].

Then I think this deserves a unit test to assert the default configuration from the simplest examples (either via the flask extension or the function decorator) do enforce a same-origin policy by default.

Also, documentation should be reviewed because some examples won't make sense. And maybe updated to emphasize everybody should whitelist specific domains and avoid "*".

IMHO raising a warning if someone configures a wildcard origin would not be out of scope. I guess the right place for such check would be the flask_cors.core.serialize_options function. But this is arguable.

Later, package major version should be bumped when releasing this.

I am unaware of further considerations.

corydolphin commented 4 months ago

Hello,

First, I'd love to have more maintainers who can help to continue to support this project since I do not use Flask on a regular cadence at this point. Second, I'd love to see a PR which changed the behavior and bumped the major version, I'd happily approve it and help support the release

Helveg commented 4 months ago

I took a look at the open issues and pull requests, and I'm willing to triage wherever I can!