Skip to content

Commit 8c015b0

Browse files
tausbntausbn
authored
Merge pull request github#17305 from Kwstubbs/CORSMiddleware-Starlette
Python: Add Support for CORS Middlewares
2 parents 4795333 + 01aa63e commit 8c015b0

File tree

15 files changed

+389
-1
lines changed

15 files changed

+389
-1
lines changed

‎python/ql/lib/semmle/python/Concepts.qll

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,6 +1411,59 @@ module Http {
14111411
override DataFlow::Node getValueArg() { none() }
14121412
}
14131413

1414+
/**
1415+
* A data-flow node that enables or disables CORS
1416+
* in a global manner.
1417+
*
1418+
* Extend this class to refine existing API models. If you want to model new APIs,
1419+
* extend `CorsMiddleware::Range` instead.
1420+
*/
1421+
class CorsMiddleware extends DataFlow::Node instanceof CorsMiddleware::Range {
1422+
/**
1423+
* Gets the string corresponding to the middleware
1424+
*/
1425+
string getMiddlewareName() { result = super.getMiddlewareName() }
1426+
1427+
/**
1428+
* Gets the dataflow node corresponding to the allowed CORS origins
1429+
*/
1430+
DataFlow::Node getOrigins() { result = super.getOrigins() }
1431+
1432+
/**
1433+
* Gets the boolean value corresponding to if CORS credentials is enabled
1434+
* (`true`) or disabled (`false`) by this node.
1435+
*/
1436+
DataFlow::Node getCredentialsAllowed() { result = super.getCredentialsAllowed() }
1437+
}
1438+
1439+
/** Provides a class for modeling new CORS middleware APIs. */
1440+
module CorsMiddleware {
1441+
/**
1442+
* A data-flow node that enables or disables Cross-site request forgery protection
1443+
* in a global manner.
1444+
*
1445+
* Extend this class to model new APIs. If you want to refine existing API models,
1446+
* extend `CorsMiddleware` instead.
1447+
*/
1448+
abstract class Range extends DataFlow::Node {
1449+
/**
1450+
* Gets the name corresponding to the middleware
1451+
*/
1452+
abstract string getMiddlewareName();
1453+
1454+
/**
1455+
* Gets the strings corresponding to the origins allowed by the cors policy
1456+
*/
1457+
abstract DataFlow::Node getOrigins();
1458+
1459+
/**
1460+
* Gets the boolean value corresponding to if CORS credentials is enabled
1461+
* (`true`) or disabled (`false`) by this node.
1462+
*/
1463+
abstract DataFlow::Node getCredentialsAllowed();
1464+
}
1465+
}
1466+
14141467
/**
14151468
* A data-flow node that enables or disables Cross-site request forgery protection
14161469
* in a global manner.

‎python/ql/lib/semmle/python/frameworks/FastApi.qll

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,51 @@ module FastApi {
3030
API::Node instance() { result = cls().getReturn() }
3131
}
3232

33+
/**
34+
* A call to `app.add_middleware` adding a generic middleware.
35+
*/
36+
private class AddMiddlewareCall extends DataFlow::CallCfgNode {
37+
AddMiddlewareCall() { this = App::instance().getMember("add_middleware").getACall() }
38+
39+
/**
40+
* Gets the string corresponding to the middleware
41+
*/
42+
string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
43+
}
44+
45+
/**
46+
* A call to `app.add_middleware` adding CORSMiddleware.
47+
*/
48+
class AddCorsMiddlewareCall extends Http::Server::CorsMiddleware::Range, AddMiddlewareCall {
49+
/**
50+
* Gets the string corresponding to the middleware
51+
*/
52+
override string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
53+
54+
/**
55+
* Gets the dataflow node corresponding to the allowed CORS origins
56+
*/
57+
override DataFlow::Node getOrigins() { result = this.getArgByName("allow_origins") }
58+
59+
/**
60+
* Gets the boolean value corresponding to if CORS credentials is enabled
61+
* (`true`) or disabled (`false`) by this node.
62+
*/
63+
override DataFlow::Node getCredentialsAllowed() {
64+
result = this.getArgByName("allow_credentials")
65+
}
66+
67+
/**
68+
* Gets the dataflow node corresponding to the allowed CORS methods
69+
*/
70+
DataFlow::Node getMethods() { result = this.getArgByName("allow_methods") }
71+
72+
/**
73+
* Gets the dataflow node corresponding to the allowed CORS headers
74+
*/
75+
DataFlow::Node getHeaders() { result = this.getArgByName("allow_headers") }
76+
}
77+
3378
/**
3479
* Provides models for the `fastapi.APIRouter` class
3580
*

‎python/ql/lib/semmle/python/frameworks/Starlette.qll

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,74 @@ private import semmle.python.frameworks.data.ModelsAsData
2525
* - https://www.starlette.io/
2626
*/
2727
module Starlette {
28+
/**
29+
* Provides models for the `starlette.app` class
30+
*/
31+
module App {
32+
/** Gets import of `starlette.app`. */
33+
API::Node cls() { result = API::moduleImport("starlette").getMember("app") }
34+
35+
/** Gets a reference to a Starlette application (an instance of `starlette.app`). */
36+
API::Node instance() { result = cls().getAnInstance() }
37+
}
38+
39+
/**
40+
* A call to any of the execute methods on a `app.add_middleware`.
41+
*/
42+
class AddMiddlewareCall extends DataFlow::CallCfgNode {
43+
AddMiddlewareCall() {
44+
this = [App::instance().getMember("add_middleware").getACall(), Middleware::instance()]
45+
}
46+
47+
/**
48+
* Gets the string corresponding to the middleware
49+
*/
50+
string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
51+
}
52+
53+
/**
54+
* A call to any of the execute methods on a `app.add_middleware` with CORSMiddleware.
55+
*/
56+
class AddCorsMiddlewareCall extends AddMiddlewareCall, Http::Server::CorsMiddleware::Range {
57+
/**
58+
* Gets the string corresponding to the middleware
59+
*/
60+
override string getMiddlewareName() { result = this.getArg(0).asExpr().(Name).getId() }
61+
62+
override DataFlow::Node getOrigins() { result = this.getArgByName("allow_origins") }
63+
64+
override DataFlow::Node getCredentialsAllowed() {
65+
result = this.getArgByName("allow_credentials")
66+
}
67+
68+
/**
69+
* Gets the dataflow node corresponding to the allowed CORS methods
70+
*/
71+
DataFlow::Node getMethods() { result = this.getArgByName("allow_methods") }
72+
73+
/**
74+
* Gets the dataflow node corresponding to the allowed CORS headers
75+
*/
76+
DataFlow::Node getHeaders() { result = this.getArgByName("allow_headers") }
77+
}
78+
79+
/**
80+
* Provides models for the `starlette.middleware.Middleware` class
81+
*
82+
* See https://www.starlette.io/.
83+
*/
84+
module Middleware {
85+
/** Gets a reference to the `starlette.middleware.Middleware` class. */
86+
API::Node classRef() {
87+
result = API::moduleImport("starlette").getMember("middleware").getMember("Middleware")
88+
or
89+
result = ModelOutput::getATypeNode("starlette.middleware.Middleware~Subclass").getASubclass*()
90+
}
91+
92+
/** Gets a reference to an instance of `starlette.middleware.Middleware`. */
93+
DataFlow::Node instance() { result = classRef().getACall() }
94+
}
95+
2896
/**
2997
* Provides models for the `starlette.websockets.WebSocket` class
3098
*

‎python/ql/src/change-notes/2024-08-26-Cors-misconfiguration-middleware.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The `py/cors-misconfiguration-with-credentials` query, which finds insecure CORS middleware configurations.

‎python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Web browsers, by default, disallow cross-origin resource sharing via direct HTTP requests.
8+
Still, to satisfy some needs that arose with the growth of the web, an expedient was created to make exceptions possible.
9+
CORS (Cross-origin resource sharing) is a mechanism that allows resources of a web endpoint (let's call it "Peer A")
10+
to be accessed from another web page belonging to a different domain ("Peer B").
11+
</p>
12+
<p>
13+
For that to happen, Peer A needs to make available its CORS configuration via special headers on the desired endpoint
14+
via the OPTIONS method.
15+
</p>
16+
<p>
17+
This configuration can also allow the inclusion of cookies on the cross-origin request,
18+
(i.e. when the <code>Access-Control-Allow-Credentials</code> header is set to true)
19+
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.
20+
</p>
21+
<p>
22+
That can have dangerous effects if the origin of Peer B is not restricted correctly.
23+
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
24+
(and not correctly validated), or is set to special values such as <code>*</code> or <code>null</code>.
25+
The above values can allow any Peer B to send requests to the misconfigured Peer A on behalf of the user.
26+
</p>
27+
<p>
28+
Example scenario:
29+
User is client of a bank that has its API misconfigured to accept CORS requests from any domain.
30+
When the user loads an evil page, the evil page sends a request to the bank's API to transfer all funds
31+
to evil party's account.
32+
Given that the user was already logged in to the bank website, and had its session cookies set,
33+
the evil party's request succeeds.
34+
</p>
35+
</overview>
36+
<recommendation>
37+
<p>
38+
When configuring CORS that allow credentials passing,
39+
it's best not to use user-provided values for the allowed origins response header,
40+
especially if the cookies grant session permissions on the user's account.
41+
</p>
42+
<p>
43+
It also can be very dangerous to set the allowed origins to <code>null</code> (which can be bypassed).
44+
</p>
45+
</recommendation>
46+
<example>
47+
<p>
48+
The first example shows a possible CORS misconfiguration case:
49+
</p>
50+
<sample src="CorsMisconfigurationMiddlewareBad.py"/>
51+
<p>
52+
The second example shows a better configuration:
53+
</p>
54+
<sample src="CorsMisconfigurationMiddlewareGood.py"/>
55+
</example>
56+
<references>
57+
<li>
58+
Reference 1: <a href="https://portswigger.net/web-security/cors">PortSwigger Web Security Academy on CORS</a>.
59+
</li>
60+
<li>
61+
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>.
62+
</li>
63+
</references>
64+
</qhelp>

‎python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @name Cors misconfiguration with credentials
3+
* @description Disabling or weakening SOP protection may make the application
4+
* vulnerable to a CORS attack.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id py/cors-misconfiguration-with-credentials
10+
* @tags security
11+
* external/cwe/cwe-942
12+
*/
13+
14+
import python
15+
import semmle.python.Concepts
16+
private import semmle.python.dataflow.new.DataFlow
17+
18+
predicate containsStar(DataFlow::Node array) {
19+
array.asExpr() instanceof List and
20+
array.asExpr().getASubExpression().(StringLiteral).getText() in ["*", "null"]
21+
or
22+
array.asExpr().(StringLiteral).getText() in ["*", "null"]
23+
}
24+
25+
predicate isCorsMiddleware(Http::Server::CorsMiddleware middleware) {
26+
middleware.getMiddlewareName() = "CORSMiddleware"
27+
}
28+
29+
predicate credentialsAllowed(Http::Server::CorsMiddleware middleware) {
30+
middleware.getCredentialsAllowed().asExpr() instanceof True
31+
}
32+
33+
from Http::Server::CorsMiddleware a
34+
where
35+
credentialsAllowed(a) and
36+
containsStar(a.getOrigins().getALocalSource()) and
37+
isCorsMiddleware(a)
38+
select a,
39+
"This CORS middleware uses a vulnerable configuration that allows arbitrary websites to make authenticated cross-site requests"

‎python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from fastapi import FastAPI
2+
from fastapi.middleware.cors import CORSMiddleware
3+
4+
app = FastAPI()
5+
6+
origins = [
7+
"*"
8+
]
9+
10+
app.add_middleware(
11+
CORSMiddleware,
12+
allow_origins=origins,
13+
allow_credentials=True,
14+
allow_methods=["*"],
15+
allow_headers=["*"],
16+
)
17+
18+
19+
@app.get("/")
20+
async def main():
21+
return {"message": "Hello World"}

‎python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
from fastapi import FastAPI
2+
from fastapi.middleware.cors import CORSMiddleware
3+
4+
app = FastAPI()
5+
6+
origins = [
7+
"http://localhost.tiangolo.com",
8+
"https://localhost.tiangolo.com",
9+
"http://localhost",
10+
"http://localhost:8080",
11+
]
12+
13+
app.add_middleware(
14+
CORSMiddleware,
15+
allow_origins=origins,
16+
allow_credentials=True,
17+
allow_methods=["*"],
18+
allow_headers=["*"],
19+
)
20+
21+
22+
@app.get("/")
23+
async def main():
24+
return {"message": "Hello World"}

‎python/ql/test/experimental/meta/ConceptsTest.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,13 +632,27 @@ module XmlParsingTest implements TestSig {
632632
}
633633
}
634634

635+
module CorsMiddlewareTest implements TestSig {
636+
string getARelevantTag() { result = "CorsMiddleware" }
637+
638+
predicate hasActualResult(Location location, string element, string tag, string value) {
639+
exists(location.getFile().getRelativePath()) and
640+
exists(Http::Server::CorsMiddleware cm |
641+
location = cm.getLocation() and
642+
element = cm.toString() and
643+
value = cm.getMiddlewareName().toString() and
644+
tag = "CorsMiddleware"
645+
)
646+
}
647+
}
648+
635649
import MakeTest<MergeTests5<MergeTests5<SystemCommandExecutionTest, DecodingTest, EncodingTest, LoggingTest,
636650
CodeExecutionTest>,
637651
MergeTests5<SqlConstructionTest, SqlExecutionTest, XPathConstructionTest, XPathExecutionTest,
638652
EscapingTest>,
639653
MergeTests5<HttpServerRouteSetupTest, HttpServerRequestHandlerTest, HttpServerHttpResponseTest,
640654
HttpServerHttpRedirectResponseTest,
641-
MergeTests<HttpServerCookieWriteTest, HttpResponseHeaderWriteTest>>,
655+
MergeTests3<HttpServerCookieWriteTest, HttpResponseHeaderWriteTest, CorsMiddlewareTest>>,
642656
MergeTests5<FileSystemAccessTest, FileSystemWriteAccessTest, PathNormalizationTest,
643657
SafeAccessCheckTest, PublicKeyGenerationTest>,
644658
MergeTests5<CryptographicOperationTest, HttpClientRequestTest, CsrfProtectionSettingTest,

‎python/ql/test/library-tests/frameworks/fastapi/middleware.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
from fastapi import FastAPI
2+
from fastapi.middleware.cors import CORSMiddleware
3+
4+
app = FastAPI()
5+
6+
origins = [
7+
"*"
8+
]
9+
10+
app.add_middleware(CORSMiddleware, allow_origins=origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"]) # $ CorsMiddleware=CORSMiddleware

0 commit comments

Comments
 (0)