-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enforcing same origin policy with flask-cors #320
Comments
This is something that would make a great pull request ;) |
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 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! |
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. |
To get this Same Origin behaviour, I should grab the host and protocol as the default value for 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? 😂 |
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. |
Hey folks, Loosely held opinions open to change. First, everything above is probably accurate.
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? |
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 * 👍 |
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. |
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. |
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? |
So what needs to be done to pull the trigger on this @corydolphin ? I have time to PR it. |
AFAIU it boils down to simply replacing the 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 Later, package major version should be bumped when releasing this. I am unaware of further considerations. |
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 |
I took a look at the open issues and pull requests, and I'm willing to triage wherever I can! |
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?
The text was updated successfully, but these errors were encountered: