diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 75b884a9dd4..b6f540373a5 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -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. diff --git a/python/ql/lib/semmle/python/frameworks/FastApi.qll b/python/ql/lib/semmle/python/frameworks/FastApi.qll index 3a15ca6fbcc..ed28f18a85a 100644 --- a/python/ql/lib/semmle/python/frameworks/FastApi.qll +++ b/python/ql/lib/semmle/python/frameworks/FastApi.qll @@ -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 * diff --git a/python/ql/lib/semmle/python/frameworks/Starlette.qll b/python/ql/lib/semmle/python/frameworks/Starlette.qll index ec62888ecb0..7ced137fcfc 100644 --- a/python/ql/lib/semmle/python/frameworks/Starlette.qll +++ b/python/ql/lib/semmle/python/frameworks/Starlette.qll @@ -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 * diff --git a/python/ql/src/change-notes/2024-08-26-Cors-misconfiguration-middleware.md b/python/ql/src/change-notes/2024-08-26-Cors-misconfiguration-middleware.md new file mode 100644 index 00000000000..aa8bc7198b3 --- /dev/null +++ b/python/ql/src/change-notes/2024-08-26-Cors-misconfiguration-middleware.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* The `py/cors-misconfiguration-with-credentials` query, which finds insecure CORS middleware configurations. \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp new file mode 100644 index 00000000000..21a670019c3 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp @@ -0,0 +1,64 @@ + + + +

+ 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"). +

+

+ For that to happen, Peer A needs to make available its CORS configuration via special headers on the desired endpoint + via the OPTIONS method. +

+

+ This configuration can also allow the inclusion of cookies on the cross-origin request, + (i.e. when the Access-Control-Allow-Credentials 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. +

+

+ That can have dangerous effects if the origin of Peer B is not restricted correctly. + An example of a dangerous scenario is when Access-Control-Allow-Origin 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 * or null. + The above values can allow any Peer B to send requests to the misconfigured Peer A on behalf of the user. +

+

+ 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. +

+ + +

+ 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. +

+

+ It also can be very dangerous to set the allowed origins to null (which can be bypassed). +

+ + +

+ The first example shows a possible CORS misconfiguration case: +

+ +

+ The second example shows a better configuration: +

+ + + +
  • + Reference 1: PortSwigger Web Security Academy on CORS. +
    • +
  • + Reference 2: AppSec EU 2017 Exploiting CORS Misconfigurations For Bitcoins And Bounties by James Kettle. +
    • +     +     \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql new file mode 100644 index 00000000000..9d2b461b986 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql @@ -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" diff --git a/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py new file mode 100644 index 00000000000..2b1c3c58a7e --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py @@ -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"} \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py new file mode 100644 index 00000000000..bfcb559feb2 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py @@ -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"} \ No newline at end of file diff --git a/python/ql/test/experimental/meta/ConceptsTest.qll b/python/ql/test/experimental/meta/ConceptsTest.qll index a53171de88a..05d60f92e21 100644 --- a/python/ql/test/experimental/meta/ConceptsTest.qll +++ b/python/ql/test/experimental/meta/ConceptsTest.qll @@ -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>, + MergeTests3>, MergeTests5, MergeTests5