Skip to content

Commit

Permalink
Merge pull request #17305 from Kwstubbs/CORSMiddleware-Starlette
Browse files Browse the repository at this point in the history
Python: Add Support for CORS Middlewares
  • Loading branch information
tausbn authored Sep 24, 2024
2 parents 4795333 + 01aa63e commit 8c015b0
Show file tree
Hide file tree
Showing 15 changed files with 389 additions and 1 deletion.
53 changes: 53 additions & 0 deletions python/ql/lib/semmle/python/Concepts.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,59 @@ module Http {
override DataFlow::Node getValueArg() { none() }
}

/**
* A data-flow node that enables or disables CORS
* in a global manner.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `CorsMiddleware::Range` instead.
*/
class CorsMiddleware extends DataFlow::Node instanceof CorsMiddleware::Range {
/**
* Gets the string corresponding to the middleware
*/
string getMiddlewareName() { result = super.getMiddlewareName() }

/**
* Gets the dataflow node corresponding to the allowed CORS origins
*/
DataFlow::Node getOrigins() { result = super.getOrigins() }

/**
* Gets the boolean value corresponding to if CORS credentials is enabled
* (`true`) or disabled (`false`) by this node.
*/
DataFlow::Node getCredentialsAllowed() { result = super.getCredentialsAllowed() }
}

/** Provides a class for modeling new CORS middleware APIs. */
module CorsMiddleware {
/**
* A data-flow node that enables or disables Cross-site request forgery protection
* in a global manner.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `CorsMiddleware` instead.
*/
abstract class Range extends DataFlow::Node {
/**
* Gets the name corresponding to the middleware
*/
abstract string getMiddlewareName();

/**
* Gets the strings corresponding to the origins allowed by the cors policy
*/
abstract DataFlow::Node getOrigins();

/**
* Gets the boolean value corresponding to if CORS credentials is enabled
* (`true`) or disabled (`false`) by this node.
*/
abstract DataFlow::Node getCredentialsAllowed();
}
}

/**
* A data-flow node that enables or disables Cross-site request forgery protection
* in a global manner.
Expand Down
45 changes: 45 additions & 0 deletions python/ql/lib/semmle/python/frameworks/FastApi.qll
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,51 @@ module FastApi {
API::Node instance() { result = cls().getReturn() }
}

/**
* A call to `app.add_middleware` adding a generic middleware.
*/
private class AddMiddlewareCall extends DataFlow::CallCfgNode {
AddMiddlewareCall() { this = App::instance().getMember("add_middleware").getACall() }

/**
* Gets the string corresponding to the middleware
*/
string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
}

/**
* A call to `app.add_middleware` adding CORSMiddleware.
*/
class AddCorsMiddlewareCall extends Http::Server::CorsMiddleware::Range, AddMiddlewareCall {
/**
* Gets the string corresponding to the middleware
*/
override string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }

/**
* Gets the dataflow node corresponding to the allowed CORS origins
*/
override DataFlow::Node getOrigins() { result = this.getArgByName("allow_origins") }

/**
* Gets the boolean value corresponding to if CORS credentials is enabled
* (`true`) or disabled (`false`) by this node.
*/
override DataFlow::Node getCredentialsAllowed() {
result = this.getArgByName("allow_credentials")
}

/**
* Gets the dataflow node corresponding to the allowed CORS methods
*/
DataFlow::Node getMethods() { result = this.getArgByName("allow_methods") }

/**
* Gets the dataflow node corresponding to the allowed CORS headers
*/
DataFlow::Node getHeaders() { result = this.getArgByName("allow_headers") }
}

/**
* Provides models for the `fastapi.APIRouter` class
*
Expand Down
68 changes: 68 additions & 0 deletions python/ql/lib/semmle/python/frameworks/Starlette.qll
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,74 @@ private import semmle.python.frameworks.data.ModelsAsData
* - https://www.starlette.io/
*/
module Starlette {
/**
* Provides models for the `starlette.app` class
*/
module App {
/** Gets import of `starlette.app`. */
API::Node cls() { result = API::moduleImport("starlette").getMember("app") }

/** Gets a reference to a Starlette application (an instance of `starlette.app`). */
API::Node instance() { result = cls().getAnInstance() }
}

/**
* A call to any of the execute methods on a `app.add_middleware`.
*/
class AddMiddlewareCall extends DataFlow::CallCfgNode {
AddMiddlewareCall() {
this = [App::instance().getMember("add_middleware").getACall(), Middleware::instance()]
}

/**
* Gets the string corresponding to the middleware
*/
string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
}

/**
* A call to any of the execute methods on a `app.add_middleware` with CORSMiddleware.
*/
class AddCorsMiddlewareCall extends AddMiddlewareCall, Http::Server::CorsMiddleware::Range {
/**
* Gets the string corresponding to the middleware
*/
override string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }

override DataFlow::Node getOrigins() { result = this.getArgByName("allow_origins") }

override DataFlow::Node getCredentialsAllowed() {
result = this.getArgByName("allow_credentials")
}

/**
* Gets the dataflow node corresponding to the allowed CORS methods
*/
DataFlow::Node getMethods() { result = this.getArgByName("allow_methods") }

/**
* Gets the dataflow node corresponding to the allowed CORS headers
*/
DataFlow::Node getHeaders() { result = this.getArgByName("allow_headers") }
}

/**
* Provides models for the `starlette.middleware.Middleware` class
*
* See https://www.starlette.io/.
*/
module Middleware {
/** Gets a reference to the `starlette.middleware.Middleware` class. */
API::Node classRef() {
result = API::moduleImport("starlette").getMember("middleware").getMember("Middleware")
or
result = ModelOutput::getATypeNode("starlette.middleware.Middleware~Subclass").getASubclass*()
}

/** Gets a reference to an instance of `starlette.middleware.Middleware`. */
DataFlow::Node instance() { result = classRef().getACall() }
}

/**
* Provides models for the `starlette.websockets.WebSocket` class
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: newQuery
---
* The `py/cors-misconfiguration-with-credentials` query, which finds insecure CORS middleware configurations.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Web browsers, by default, disallow cross-origin resource sharing via direct HTTP requests.
Still, to satisfy some needs that arose with the growth of the web, an expedient was created to make exceptions possible.
CORS (Cross-origin resource sharing) is a mechanism that allows resources of a web endpoint (let's call it "Peer A")
to be accessed from another web page belonging to a different domain ("Peer B").
</p>
<p>
For that to happen, Peer A needs to make available its CORS configuration via special headers on the desired endpoint
via the OPTIONS method.
</p>
<p>
This configuration can also allow the inclusion of cookies on the cross-origin request,
(i.e. when the <code>Access-Control-Allow-Credentials</code> header is set to true)
meaning that Peer B can send a request to Peer A that will include the cookies as if the request was executed by the user.
</p>
<p>
That can have dangerous effects if the origin of Peer B is not restricted correctly.
An example of a dangerous scenario is when <code>Access-Control-Allow-Origin</code> header is set to a value obtained from the request made by Peer B
(and not correctly validated), or is set to special values such as <code>*</code> or <code>null</code>.
The above values can allow any Peer B to send requests to the misconfigured Peer A on behalf of the user.
</p>
<p>
Example scenario:
User is client of a bank that has its API misconfigured to accept CORS requests from any domain.
When the user loads an evil page, the evil page sends a request to the bank's API to transfer all funds
to evil party's account.
Given that the user was already logged in to the bank website, and had its session cookies set,
the evil party's request succeeds.
</p>
</overview>
<recommendation>
<p>
When configuring CORS that allow credentials passing,
it's best not to use user-provided values for the allowed origins response header,
especially if the cookies grant session permissions on the user's account.
</p>
<p>
It also can be very dangerous to set the allowed origins to <code>null</code> (which can be bypassed).
</p>
</recommendation>
<example>
<p>
The first example shows a possible CORS misconfiguration case:
</p>
<sample src="CorsMisconfigurationMiddlewareBad.py"/>
<p>
The second example shows a better configuration:
</p>
<sample src="CorsMisconfigurationMiddlewareGood.py"/>
</example>
<references>
<li>
Reference 1: <a href="https://portswigger.net/web-security/cors">PortSwigger Web Security Academy on CORS</a>.
</li>
<li>
Reference 2: <a href="https://www.youtube.com/watch?v=wgkj4ZgxI4c">AppSec EU 2017 Exploiting CORS Misconfigurations For Bitcoins And Bounties by James Kettle</a>.
</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @name Cors misconfiguration with credentials
* @description Disabling or weakening SOP protection may make the application
* vulnerable to a CORS attack.
* @kind problem
* @problem.severity warning
* @security-severity 8.8
* @precision high
* @id py/cors-misconfiguration-with-credentials
* @tags security
* external/cwe/cwe-942
*/

import python
import semmle.python.Concepts
private import semmle.python.dataflow.new.DataFlow

predicate containsStar(DataFlow::Node array) {
array.asExpr() instanceof List and
array.asExpr().getASubExpression().(StringLiteral).getText() in ["*", "null"]
or
array.asExpr().(StringLiteral).getText() in ["*", "null"]
}

predicate isCorsMiddleware(Http::Server::CorsMiddleware middleware) {
middleware.getMiddlewareName() = "CORSMiddleware"
}

predicate credentialsAllowed(Http::Server::CorsMiddleware middleware) {
middleware.getCredentialsAllowed().asExpr() instanceof True
}

from Http::Server::CorsMiddleware a
where
credentialsAllowed(a) and
containsStar(a.getOrigins().getALocalSource()) and
isCorsMiddleware(a)
select a,
"This CORS middleware uses a vulnerable configuration that allows arbitrary websites to make authenticated cross-site requests"
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware

app = FastAPI()

origins = [
"*"
]

app.add_middleware(
CORSMiddleware,
allow_origins=origins,
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)


@app.get("/")
async def main():
return {"message": "Hello World"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware

app = FastAPI()

origins = [
"http://localhost.tiangolo.com",
"https://localhost.tiangolo.com",
"http://localhost",
"http://localhost:8080",
]

app.add_middleware(
CORSMiddleware,
allow_origins=origins,
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)


@app.get("/")
async def main():
return {"message": "Hello World"}
16 changes: 15 additions & 1 deletion python/ql/test/experimental/meta/ConceptsTest.qll
Original file line number Diff line number Diff line change
Expand Up @@ -632,13 +632,27 @@ module XmlParsingTest implements TestSig {
}
}

module CorsMiddlewareTest implements TestSig {
string getARelevantTag() { result = "CorsMiddleware" }

predicate hasActualResult(Location location, string element, string tag, string value) {
exists(location.getFile().getRelativePath()) and
exists(Http::Server::CorsMiddleware cm |
location = cm.getLocation() and
element = cm.toString() and
value = cm.getMiddlewareName().toString() and
tag = "CorsMiddleware"
)
}
}

import MakeTest<MergeTests5<MergeTests5<SystemCommandExecutionTest, DecodingTest, EncodingTest, LoggingTest,
CodeExecutionTest>,
MergeTests5<SqlConstructionTest, SqlExecutionTest, XPathConstructionTest, XPathExecutionTest,
EscapingTest>,
MergeTests5<HttpServerRouteSetupTest, HttpServerRequestHandlerTest, HttpServerHttpResponseTest,
HttpServerHttpRedirectResponseTest,
MergeTests<HttpServerCookieWriteTest, HttpResponseHeaderWriteTest>>,
MergeTests3<HttpServerCookieWriteTest, HttpResponseHeaderWriteTest, CorsMiddlewareTest>>,
MergeTests5<FileSystemAccessTest, FileSystemWriteAccessTest, PathNormalizationTest,
SafeAccessCheckTest, PublicKeyGenerationTest>,
MergeTests5<CryptographicOperationTest, HttpClientRequestTest, CsrfProtectionSettingTest,
Expand Down
10 changes: 10 additions & 0 deletions python/ql/test/library-tests/frameworks/fastapi/middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware

app = FastAPI()

origins = [
"*"
]

app.add_middleware(CORSMiddleware, allow_origins=origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"]) # $ CorsMiddleware=CORSMiddleware
Loading

0 comments on commit 8c015b0

Please sign in to comment.