Skip to content
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

WIP: Python: CORS Bypass #16814

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Conversation

porcupineyhairs
Copy link
Contributor

This PR adds a query to detect a Cross Origin Resource Sharing(CORS) policy bypass due to an incorrect check.

This PR attempts to detect the vulnerability pattern found in CVE-2022-3457

if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
    origin = request.headers.get('Origin', None)
    if origin and not origin.startswith(request.base):
        raise cherrypy.HTTPError(403, 'Unexpected Origin header')

In this case, a value obtained from a header is compared using startswith call. This comparision is easily bypassed resulting in a CORS bypass. Given that similar bugs have been found in other languages as well, I think this PR would be a great addition to the exisitng python query pack.

The databases for CVE-2022-3457 can be downloaded from

https://filetransfer.io/data-package/i4Mfepls#link
https://file.io/V67T4SSgmExF

@ghsecuritylab
Copy link
Collaborator

Hello porcupineyhairs 👋
You have submitted this pull request as a bug bounty report in the github/securitylab repository and therefore this pull request has been put into draft state to give time for the GitHub Security Lab to assess the PR. When GitHub Security Lab has finished assessing your pull request, it will be marked automatically as Ready for review. Until then, please don't change the draft state.

In the meantime, feel free to make changes to the pull request. If you'd like to maximize payout for your this and future submissions, here are a few general guidelines, that we might take into consideration when reviewing a submission.

  • the submission models widely-used frameworks/libraries
  • the vulnerability modeled in the submission is impactful
  • the submission finds new true positive vulnerabilities
  • the submission finds very few false positives
  • code in the submission is easy to read and will be easy to maintain
  • documentation is written clearly, highlighting the impact of the issue it finds and is written without grammatical or other errors. The code samples clearly show the vulnerability
  • the submission includes tests, change note etc.

Please note that these are guidelines, not rules. Since we have a lot of different types of submissions, the guidelines might vary for each submission.

Happy hacking!

@ghsecuritylab ghsecuritylab marked this pull request as ready for review July 1, 2024 11:22
@ghsecuritylab ghsecuritylab requested a review from a team as a code owner July 1, 2024 11:22
@porcupineyhairs
Copy link
Contributor Author

@RasmusWL The bounty application for this Pr is already closed. Do you plan on merging this soon?

Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello
In addition to the above style warnings, there are compilation errors due to the path-problem query kind expecting the nodes selected in the alert to be PathNodes rather than DataFlow::Nodes, as so:

Comment on lines 88 to 97
from DataFlow::Node source, DataFlow::Node sink
where
CorsFlow::flow(source, sink) and
(
maybeInteresting(source.asCfgNode())
or
maybeInteresting(sink.asCfgNode())
)
select source, source, sink,
"Potentially incorrect string comparision which could lead to a CORS bypass."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from DataFlow::Node source, DataFlow::Node sink
where
CorsFlow::flow(source, sink) and
(
maybeInteresting(source.asCfgNode())
or
maybeInteresting(sink.asCfgNode())
)
select source, source, sink,
"Potentially incorrect string comparision which could lead to a CORS bypass."
from CorsFlow::PathNode source, CorsFlow::PathNode sink
where
CorsFlow::flowPath(source, sink) and
(
maybeInteresting(source.getNode().asCfgNode())
or
maybeInteresting(sink.getNode().asCfgNode())
)
select source, source, sink,
"Potentially incorrect string comparison which could lead to a CORS bypass."

This PR adds a query to detect a Cross Origin Resource Sharing(CORS) policy bypass due to an incorrect check.

This PR attempts to detect the vulnerability pattern found in CVE-2022-3457

```python
if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
    origin = request.headers.get('Origin', None)
    if origin and not origin.startswith(request.base):
        raise cherrypy.HTTPError(403, 'Unexpected Origin header')
```

In this case, a value obtained from a header is compared using `startswith` call. This comparision is easily bypassed resulting in a CORS bypass. Given that similar bugs have been found in other languages as well, I think this PR would be a great addition to the exisitng python query pack.

The databases for CVE-2022-3457 can be downloaded from
```
https://filetransfer.io/data-package/i4Mfepls#link
https://file.io/V67T4SSgmExF
```
@porcupineyhairs
Copy link
Contributor Author

@joefarebrother Sorry for the delay. Changes done. I have also included Qhelp and tests now.

Copy link
Contributor

github-actions bot commented Sep 3, 2024

QHelp previews:

python/ql/src/experimental/Security/CWE-346/CorsBypass.qhelp

Cross Origin Resource Sharing(CORS) Policy Bypass

Cross-origin resource sharing policy may be bypassed due to incorrect checks like the string.startswith call.

Recommendation

Use a more stronger check to test for CORS policy bypass.

Example

Most Python frameworks provide a mechanism for testing origins and performing CORS checks. For example, consider the code snippet below, origin is compared using a startswith call against a list of whitelisted origins. This check can be bypassed easily by origin like domain.com.baddomain.com

import cherrypy

def bad():
    request = cherrypy.request
    validCors = "domain.com"
    if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
        origin = request.headers.get('Origin', None)
        if origin.startswith(validCors):
            print("Origin Valid")

This can be prevented by comparing the origin in a manner shown below.

import cherrypy

def good():
    request = cherrypy.request
    validOrigin = "domain.com"
    if request.method in ['POST', 'PUT', 'PATCH', 'DELETE']:
        origin = request.headers.get('Origin', None)
        if origin == validOrigin:
            print("Origin Valid")

References

  • PortsSwigger : Cross-origin resource sharing (CORS)
  • Related CVE: CVE-2022-3457.

@joefarebrother joefarebrother merged commit 959715a into github:main Sep 5, 2024
13 checks passed
@porcupineyhairs porcupineyhairs deleted the pyCors branch September 5, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants