From 8bf8893307da1314158fdfbf3c24100cbeb5902f Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Mon, 26 Aug 2024 21:30:48 -0700 Subject: [PATCH] Add support for vulnerable CORS middlewares --- python/ql/lib/semmle/python/Concepts.qll | 50 ++++++++++++ .../lib/semmle/python/frameworks/FastApi.qll | 26 ++++++ .../semmle/python/frameworks/Starlette.qll | 81 +++++++++++++++++++ .../CorsMisconfigurationMiddleware.qhelp | 64 +++++++++++++++ .../CWE-942/CorsMisconfigurationMiddleware.ql | 36 +++++++++ .../CorsMisconfigurationMiddlewareBad.py | 21 +++++ .../CorsMisconfigurationMiddlewareGood.py | 24 ++++++ .../CorsMisconfigurationMiddleware.expected | 2 + .../CorsMisconfigurationMiddleware.qlref | 1 + .../fastapi.py | 21 +++++ .../starlette.py | 11 +++ 11 files changed, 337 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.qhelp create mode 100644 python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql create mode 100644 python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareBad.py create mode 100644 python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddlewareGood.py create mode 100644 python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.expected create mode 100644 python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.qlref create mode 100644 python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/fastapi.py create mode 100644 python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/starlette.py diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 75b884a9dd4..920fbca3553 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -1411,6 +1411,56 @@ 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 middleware_name() { result = super.middleware_name() } + + /** + * Gets the boolean value corresponding to if CORS credentials is enabled + * (`true`) or disabled (`false`) by this node. + */ + DataFlow::Node allowed_origins() { result = super.allowed_origins() } + + DataFlow::Node allowed_credentials() { result = super.allowed_credentials() } + } + + /** 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 string corresponding to the middleware + */ + abstract string middleware_name(); + + /** + * Gets the boolean value corresponding to if CORS credentials is enabled + * (`true`) or disabled (`false`) by this node. + */ + abstract DataFlow::Node allowed_credentials(); + + /** + * Gets the strings corresponding to the origins allowed by the cors policy + */ + abstract DataFlow::Node allowed_origins(); + } + } + /** * 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..7927dac5a24 100644 --- a/python/ql/lib/semmle/python/frameworks/FastApi.qll +++ b/python/ql/lib/semmle/python/frameworks/FastApi.qll @@ -30,6 +30,32 @@ 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() } + + string middleware_name() { result = this.getArg(0).asExpr().(Name).toString() } + } + + /** + * A call to `app.add_middleware` adding CORSMiddleware. + */ + class AddCorsMiddlewareCall extends Http::Server::CorsMiddleware::Range, AddMiddlewareCall { + override string middleware_name() { result = this.getArg(0).asExpr().(Name).toString() } + + override DataFlow::Node allowed_origins() { result = this.getArgByName("allow_origins") } + + override DataFlow::Node allowed_credentials() { + result = this.getArgByName("allow_credentials") + } + + DataFlow::Node allowed_methods() { result = this.getArgByName("allow_methods") } + + DataFlow::Node allowed_headers() { 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..d54fc267b41 100644 --- a/python/ql/lib/semmle/python/frameworks/Starlette.qll +++ b/python/ql/lib/semmle/python/frameworks/Starlette.qll @@ -25,6 +25,87 @@ private import semmle.python.frameworks.data.ModelsAsData * - https://www.starlette.io/ */ module Starlette { + /** + * Provides models for the `starlette.app` class + * + * See https://www.starlette.io/websockets/. + */ + module App { + API::Node cls() { result = API::moduleImport("starlette").getMember("app") } + + /** Gets a reference to a FastAPI application (an instance of `fastapi.FastAPI`). */ + API::Node instance() { result = cls().getReturn() } + } + + /** + * 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()] + } + + string middleware_name() { result = this.getArg(0).asExpr().(Name).toString() } + } + + /** + * A call to any of the execute methods on a `app.add_middleware` with CORSMiddleware. + */ + class AddCorsMiddlewareCall extends AddMiddlewareCall, Http::Server::CorsMiddleware::Range { + override string middleware_name() { result = this.getArg(0).asExpr().(Name).toString() } + + override DataFlow::Node allowed_origins() { result = this.getArgByName("allow_origins") } + + override DataFlow::Node allowed_credentials() { + result = this.getArgByName("allow_credentials") + } + + DataFlow::Node allowed_methods() { result = this.getArgByName("allow_methods") } + + DataFlow::Node allowed_headers() { 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*() + } + + /** + * A source of instances of `starlette.middleware.Middleware`, extend this class to model new instances. + * + * This can include instantiations of the class, return values from function + * calls, or a special parameter that will be set when functions are called by an external + * library. + * + * Use the predicate `Middleware::instance()` to get references to instances of `starlette.middleware.middleware`. + */ + abstract class InstanceSource extends DataFlow::LocalSourceNode { } + + /** A direct instantiation of `starlette.middleware.Middleware`. */ + class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode { + ClassInstantiation() { this = classRef().getACall() } + } + + /** Gets a reference to an instance of `starlette.middleware.Middleware`. */ + private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { + t.start() and + result instanceof InstanceSource + or + exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) + } + + /** Gets a reference to an instance of `starlette.middleware.Middleware`. */ + DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } + } + /** * Provides models for the `starlette.websockets.WebSocket` class * 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..43f800f03a5 --- /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 (i.e. using a JavaScript HTTP client). + 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 Peer B origin is not restricted correctly. + An example of a dangerous scenario is when Access-Control-Allow-Origin header is set to a value gotten from the Peer B's request + (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 few possible CORS misconfiguration cases: +

+ +

+ The second example show better configurations: +

+ + + +
  • + 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..ddbd0766207 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql @@ -0,0 +1,36 @@ +/** + * @name SOP protection weak 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/insecure-cors-setting + * @tags security + * external/cwe/cwe-352 + */ + + 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().matches("*")) or + (array.asExpr().(StringLiteral).getText().matches(["*", "null"])) + + } + + predicate isCorsMiddleware(Http::Server::CorsMiddleware middleware){ + middleware.middleware_name().matches("CORSMiddleware") + } + + predicate credentialsAllowed(Http::Server::CorsMiddleware middleware){ + middleware.allowed_credentials().asExpr() instanceof True + } + + from Http::Server::CorsMiddleware a + where credentialsAllowed(a) and + containsStar(a.allowed_origins().getALocalSource()) and + isCorsMiddleware(a) + select a, "This CORS middleware uses a vulnerable configuration that leaves it open to attacks from arbitrary websites" \ No newline at end of file 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/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.expected b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.expected new file mode 100644 index 00000000000..cc27e83a644 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.expected @@ -0,0 +1,2 @@ +| fastapi.py:10:1:16:1 | ControlFlowNode for Attribute() | This CORS middleware uses a vulnerable configuration that leaves it open to attacks from arbitrary websites | +| starlette.py:8:5:8:75 | ControlFlowNode for Middleware() | This CORS middleware uses a vulnerable configuration that leaves it open to attacks from arbitrary websites | diff --git a/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.qlref b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.qlref new file mode 100644 index 00000000000..f7017ada0d8 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/CorsMisconfigurationMiddleware.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-942/CorsMisconfigurationMiddleware.ql \ No newline at end of file diff --git a/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/fastapi.py b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/fastapi.py new file mode 100644 index 00000000000..2b1c3c58a7e --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/fastapi.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/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/starlette.py b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/starlette.py new file mode 100644 index 00000000000..1d6e7705369 --- /dev/null +++ b/python/ql/test/query-tests/Security/CWE-942-CorsMisconfigurationMiddleware/starlette.py @@ -0,0 +1,11 @@ +from starlette.applications import Starlette +from starlette.middleware import Middleware +from starlette.middleware.cors import CORSMiddleware + +routes = ... + +middleware = [ + Middleware(CORSMiddleware, allow_origins=['*'], allow_credentials=True) +] + +app = Starlette(routes=routes, middleware=middleware) \ No newline at end of file