From 3cbb909a3a4ae8f736d10bd3e1175f1a388c6af3 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 20 May 2021 11:12:28 +0200 Subject: [PATCH] Python: Add modeling of coroutine routes in aiohttp.web --- docs/codeql/support/reusables/frameworks.rst | 1 + python/ql/src/semmle/python/Frameworks.qll | 1 + .../src/semmle/python/frameworks/Aiohttp.qll | 141 ++++++++++++++++++ .../frameworks/aiohttp/routing_test.py | 50 +++---- 4 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 python/ql/src/semmle/python/frameworks/Aiohttp.qll diff --git a/docs/codeql/support/reusables/frameworks.rst b/docs/codeql/support/reusables/frameworks.rst index be9949aad77..41680cffe0e 100644 --- a/docs/codeql/support/reusables/frameworks.rst +++ b/docs/codeql/support/reusables/frameworks.rst @@ -152,6 +152,7 @@ Python built-in support :widths: auto Name, Category + aiohttp.web, Web framework Django, Web framework Flask, Web framework Tornado, Web framework diff --git a/python/ql/src/semmle/python/Frameworks.qll b/python/ql/src/semmle/python/Frameworks.qll index 3115c3ffac6..172f3d11bfb 100644 --- a/python/ql/src/semmle/python/Frameworks.qll +++ b/python/ql/src/semmle/python/Frameworks.qll @@ -4,6 +4,7 @@ // If you add modeling of a new framework/library, remember to add it it to the docs in // `docs/codeql/support/reusables/frameworks.rst` +private import semmle.python.frameworks.Aiohttp private import semmle.python.frameworks.Cryptodome private import semmle.python.frameworks.Cryptography private import semmle.python.frameworks.Dill diff --git a/python/ql/src/semmle/python/frameworks/Aiohttp.qll b/python/ql/src/semmle/python/frameworks/Aiohttp.qll new file mode 100644 index 00000000000..b73610c06b1 --- /dev/null +++ b/python/ql/src/semmle/python/frameworks/Aiohttp.qll @@ -0,0 +1,141 @@ +/** + * Provides classes modeling security-relevant aspects of the `aiohttp` PyPI package. + * See https://docs.aiohttp.org/en/stable/index.html + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.Concepts +private import semmle.python.ApiGraphs +private import semmle.python.frameworks.internal.PoorMansFunctionResolution + +/** + * INTERNAL: Do not use. + * + * Provides models for the web server part (`aiohttp.web`) of the `aiohttp` PyPI package. + * See https://docs.aiohttp.org/en/stable/web.html + */ +module AiohttpWebModel { + /** Gets a reference to a `aiohttp.web.Application` instance. */ + API::Node applicationInstance() { + // Not sure whether you're allowed to add routes _after_ starting the app, for + // example in the middle of handling a http request... but I'm guessing that for 99% + // for all code, not modeling that `request.app` is a reference to an application + // should be good enough for the route-setup part of the modeling :+1: + result = API::moduleImport("aiohttp").getMember("web").getMember("Application").getReturn() + } + + /** Gets a reference to a `aiohttp.web.UrlDispatcher` instance. */ + API::Node urlDispathcerInstance() { + result = API::moduleImport("aiohttp").getMember("web").getMember("UrlDispatcher").getReturn() + or + result = applicationInstance().getMember("router") + } + + // -- route modeling -- + + /** A route setup in `aiohttp.web` */ + abstract class AiohttpRouteSetup extends HTTP::Server::RouteSetup::Range { + override Parameter getARoutedParameter() { none() } + + override string getFramework() { result = "aiohttp.web" } + } + + /** An aiohttp route setup that uses coroutines (async function) as request handler. */ + abstract class AiohttpCoroutineRouteSetup extends AiohttpRouteSetup { + /** Gets the argument specifying the request handler (which is a coroutine/async function) */ + abstract DataFlow::Node getRequestHandlerArg(); + + override Function getARequestHandler() { + this.getRequestHandlerArg() = poorMansFunctionTracker(result) + } + } + + /** + * A route-setup from `add_route` or any of `add_get`, `add_post`, etc. on an + * `aiohttp.web.UrlDispatcher`. + */ + class AiohttpUrlDispatcherAddRouteCall extends AiohttpCoroutineRouteSetup, DataFlow::CallCfgNode { + /** At what index route arguments starts, so we can handle "route" version together with get/post/... */ + int routeArgsStart; + + AiohttpUrlDispatcherAddRouteCall() { + this = urlDispathcerInstance().getMember("add_" + HTTP::httpVerbLower()).getACall() and + routeArgsStart = 0 + or + this = urlDispathcerInstance().getMember("add_route").getACall() and + routeArgsStart = 1 + } + + override DataFlow::Node getUrlPatternArg() { + result in [this.getArg(routeArgsStart + 0), this.getArgByName("path")] + } + + override DataFlow::Node getRequestHandlerArg() { + result in [this.getArg(routeArgsStart + 1), this.getArgByName("handler")] + } + } + + /** + * A route-setup from using `route` or any of `get`, `post`, etc. functions from `aiohttp.web`. + * + * Note that technically, this does not set up a route in itself (since it needs to be added to an application first). + * However, modeling this way makes it easier, and we don't expect it to lead to many problems. + */ + class AiohttpWebRouteCall extends AiohttpCoroutineRouteSetup, DataFlow::CallCfgNode { + /** At what index route arguments starts, so we can handle "route" version together with get/post/... */ + int routeArgsStart; + + AiohttpWebRouteCall() { + exists(string funcName | + funcName = HTTP::httpVerbLower() and + routeArgsStart = 0 + or + funcName = "route" and + routeArgsStart = 1 + | + this = API::moduleImport("aiohttp").getMember("web").getMember(funcName).getACall() + ) + } + + override DataFlow::Node getUrlPatternArg() { + result in [this.getArg(routeArgsStart + 0), this.getArgByName("path")] + } + + override DataFlow::Node getRequestHandlerArg() { + result in [this.getArg(routeArgsStart + 1), this.getArgByName("handler")] + } + } + + /** A route-setup from using a `route` or any of `get`, `post`, etc. decorators from a `aiohttp.web.RouteTableDef`. */ + class AiohttpRouteTableDefRouteCall extends AiohttpCoroutineRouteSetup, DataFlow::CallCfgNode { + /** At what index route arguments starts, so we can handle "route" version together with get/post/... */ + int routeArgsStart; + + AiohttpRouteTableDefRouteCall() { + exists(string decoratorName | + decoratorName = HTTP::httpVerbLower() and + routeArgsStart = 0 + or + decoratorName = "route" and + routeArgsStart = 1 + | + this = + API::moduleImport("aiohttp") + .getMember("web") + .getMember("RouteTableDef") + .getReturn() + .getMember(decoratorName) + .getACall() + ) + } + + override DataFlow::Node getUrlPatternArg() { + result in [this.getArg(routeArgsStart + 0), this.getArgByName("path")] + } + + override DataFlow::Node getRequestHandlerArg() { none() } + + override Function getARequestHandler() { result.getADecorator() = this.asExpr() } + } +} diff --git a/python/ql/test/library-tests/frameworks/aiohttp/routing_test.py b/python/ql/test/library-tests/frameworks/aiohttp/routing_test.py index 6a8b98cfca6..880a7f8cb87 100644 --- a/python/ql/test/library-tests/frameworks/aiohttp/routing_test.py +++ b/python/ql/test/library-tests/frameworks/aiohttp/routing_test.py @@ -15,55 +15,55 @@ app = web.Application() if True: # `app.add_routes` with list - async def foo(request): # $ MISSING: requestHandler + async def foo(request): # $ requestHandler return web.Response(text="foo") - async def foo2(request): # $ MISSING: requestHandler + async def foo2(request): # $ requestHandler return web.Response(text="foo2") - async def foo3(request): # $ MISSING: requestHandler + async def foo3(request): # $ requestHandler return web.Response(text="foo3") app.add_routes([ - web.get("/foo", foo), # $ MISSING: routeSetup - web.route("*", "/foo2", foo2), # $ MISSING: routeSetup - web.get(path="/foo3", handler=foo3), # $ MISSING: routeSetup + web.get("/foo", foo), # $ routeSetup="/foo" + web.route("*", "/foo2", foo2), # $ routeSetup="/foo2" + web.get(path="/foo3", handler=foo3), # $ routeSetup="/foo3" ]) # using decorator routes = web.RouteTableDef() - @routes.get("/bar") # $ MISSING: routeSetup - async def bar(request): # $ MISSING: requestHandler + @routes.get("/bar") # $ routeSetup="/bar" + async def bar(request): # $ requestHandler return web.Response(text="bar") - @routes.route("*", "/bar2") # $ MISSING: routeSetup - async def bar2(request): # $ MISSING: requestHandler + @routes.route("*", "/bar2") # $ routeSetup="/bar2" + async def bar2(request): # $ requestHandler return web.Response(text="bar2") - @routes.get(path="/bar3") # $ MISSING: routeSetup - async def bar3(request): # $ MISSING: requestHandler + @routes.get(path="/bar3") # $ routeSetup="/bar3" + async def bar3(request): # $ requestHandler return web.Response(text="bar3") app.add_routes(routes) # `app.router.add_get` / `app.router.add_route` - async def baz(request): # $ MISSING: requestHandler + async def baz(request): # $ requestHandler return web.Response(text="baz") - app.router.add_get("/baz", baz) # $ MISSING: routeSetup + app.router.add_get("/baz", baz) # $ routeSetup="/baz" - async def baz2(request): # $ MISSING: requestHandler + async def baz2(request): # $ requestHandler return web.Response(text="baz2") - app.router.add_route("*", "/baz2", baz2) # $ MISSING: routeSetup + app.router.add_route("*", "/baz2", baz2) # $ routeSetup="/baz2" - async def baz3(request): # $ MISSING: requestHandler + async def baz3(request): # $ requestHandler return web.Response(text="baz3") - app.router.add_get(path="/baz3", handler=baz3) # $ MISSING: routeSetup + app.router.add_get(path="/baz3", handler=baz3) # $ routeSetup="/baz3" ## Using classes / views @@ -76,7 +76,7 @@ if True: return web.Response(text="MyCustomHandlerClass.foo") my_custom_handler = MyCustomHandlerClass() - app.router.add_get("/MyCustomHandlerClass/foo", my_custom_handler.foo_handler) # $ MISSING: routeSetup + app.router.add_get("/MyCustomHandlerClass/foo", my_custom_handler.foo_handler) # $ routeSetup="/MyCustomHandlerClass/foo" # Using `web.View` # --------------- @@ -116,12 +116,12 @@ if True: if True: # see https://docs.aiohttp.org/en/stable/web_quickstart.html#variable-resources - async def matching(request: web.Request): # $ MISSING: requestHandler + async def matching(request: web.Request): # $ requestHandler name = request.match_info['name'] number = request.match_info['number'] return web.Response(text="matching name={} number={}".format(name, number)) - app.router.add_get("/matching/{name}/{number:\d+}", matching) # $ MISSING: routeSetup + app.router.add_get(r"/matching/{name}/{number:\d+}", matching) # $ routeSetup="/matching/{name}/{number:\d+}" ## ======= ## ## subapps ## @@ -130,10 +130,10 @@ if True: if True: subapp = web.Application() - async def subapp_handler(request): # $ MISSING: requestHandler + async def subapp_handler(request): # $ requestHandler return web.Response(text="subapp_handler") - subapp.router.add_get("/subapp_handler", subapp_handler) # $ MISSING: routeSetup + subapp.router.add_get("/subapp_handler", subapp_handler) # $ routeSetup="/subapp_handler" app.add_subapp("/my_subapp", subapp) @@ -146,11 +146,11 @@ if True: ## ================================ ## if True: - async def manual_dispatcher_instance(request): # $ MISSING: requestHandler + async def manual_dispatcher_instance(request): # $ requestHandler return web.Response(text="manual_dispatcher_instance") url_dispatcher = web.UrlDispatcher() - url_dispatcher.add_get("/manual_dispatcher_instance", manual_dispatcher_instance) # $ MISSING: routeSetup + url_dispatcher.add_get("/manual_dispatcher_instance", manual_dispatcher_instance) # $ routeSetup="/manual_dispatcher_instance" subapp2 = web.Application(router=url_dispatcher) app.add_subapp("/manual_dispatcher_instance_app", subapp2)