diff --git a/docs/codeql/support/reusables/frameworks.rst b/docs/codeql/support/reusables/frameworks.rst index de864197f5b3..d13d1d12a205 100644 --- a/docs/codeql/support/reusables/frameworks.rst +++ b/docs/codeql/support/reusables/frameworks.rst @@ -159,6 +159,7 @@ Python built-in support Flask, Web framework Tornado, Web framework Twisted, Web framework + Flask-Admin, Web framework starlette, Asynchronous Server Gateway Interface (ASGI) dill, Serialization PyYAML, Serialization diff --git a/python/change-notes/2021-11-02-flask_admin.md b/python/change-notes/2021-11-02-flask_admin.md new file mode 100644 index 000000000000..528a422c45de --- /dev/null +++ b/python/change-notes/2021-11-02-flask_admin.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added modeling of HTTP requests and responses when using `flask_admin` (`Flask-Admin` PyPI package), which leads to additional remote flow sources. diff --git a/python/ql/lib/semmle/python/Frameworks.qll b/python/ql/lib/semmle/python/Frameworks.qll index bc2c3291fb2e..2d8d25693a3f 100644 --- a/python/ql/lib/semmle/python/Frameworks.qll +++ b/python/ql/lib/semmle/python/Frameworks.qll @@ -15,6 +15,7 @@ private import semmle.python.frameworks.Django private import semmle.python.frameworks.Fabric private import semmle.python.frameworks.FastApi private import semmle.python.frameworks.Flask +private import semmle.python.frameworks.FlaskAdmin private import semmle.python.frameworks.FlaskSqlAlchemy private import semmle.python.frameworks.Idna private import semmle.python.frameworks.Invoke diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index ad57db65f417..88544ca85378 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/python/ql/lib/semmle/python/frameworks/Flask.qll @@ -238,7 +238,7 @@ module Flask { } /** A route setup made by flask (sharing handling of URL patterns). */ - abstract private class FlaskRouteSetup extends HTTP::Server::RouteSetup::Range { + abstract class FlaskRouteSetup extends HTTP::Server::RouteSetup::Range { override Parameter getARoutedParameter() { // If we don't know the URL pattern, we simply mark all parameters as a routed // parameter. This should give us more RemoteFlowSources but could also lead to diff --git a/python/ql/lib/semmle/python/frameworks/FlaskAdmin.qll b/python/ql/lib/semmle/python/frameworks/FlaskAdmin.qll new file mode 100644 index 000000000000..a9b90f1d9c4a --- /dev/null +++ b/python/ql/lib/semmle/python/frameworks/FlaskAdmin.qll @@ -0,0 +1,79 @@ +/** + * Provides classes modeling security-relevant aspects of the `Flask-Admin` PyPI package + * (imported as `flask_admin`). + * + * See + * - https://flask-admin.readthedocs.io/en/latest/ + * - https://pypi.org/project/Flask-Admin/ + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.dataflow.new.RemoteFlowSources +private import semmle.python.dataflow.new.TaintTracking +private import semmle.python.Concepts +private import semmle.python.frameworks.Flask +private import semmle.python.ApiGraphs + +/** + * Provides models for the `Flask-Admin` PyPI package (imported as `flask_admin`). + * + * See + * - https://flask-admin.readthedocs.io/en/latest/ + * - https://pypi.org/project/Flask-Admin/ + */ +private module FlaskAdmin { + /** + * A call to `flask_admin.expose`, which is used as a decorator to make the + * function exposed in the admin interface (and make it a request handler) + * + * See https://flask-admin.readthedocs.io/en/latest/api/mod_base/#flask_admin.base.expose + */ + private class FlaskAdminExposeCall extends Flask::FlaskRouteSetup, DataFlow::CallCfgNode { + FlaskAdminExposeCall() { + this = API::moduleImport("flask_admin").getMember("expose").getACall() + } + + override DataFlow::Node getUrlPatternArg() { + result in [this.getArg(0), this.getArgByName("url")] + } + + override Function getARequestHandler() { result.getADecorator().getAFlowNode() = node } + } + + /** + * A call to `flask_admin.expose_plugview`, which is used as a decorator to make the + * class (which we expect to be a flask View class) exposed in the admin interface. + * + * See https://flask-admin.readthedocs.io/en/latest/api/mod_base/#flask_admin.base.expose_plugview + */ + private class FlaskAdminExposePlugviewCall extends Flask::FlaskRouteSetup, DataFlow::CallCfgNode { + FlaskAdminExposePlugviewCall() { + this = API::moduleImport("flask_admin").getMember("expose_plugview").getACall() + } + + override DataFlow::Node getUrlPatternArg() { + result in [this.getArg(0), this.getArgByName("url")] + } + + override Parameter getARoutedParameter() { + result = super.getARoutedParameter() and + ( + exists(this.getUrlPattern()) + or + // the first argument is `self`, and the second argument `cls` will receive the + // containing flask_admin View class -- this is only relevant if the URL pattern + // is not known + not exists(this.getUrlPattern()) and + not result = this.getARequestHandler().getArg([0, 1]) + ) + } + + override Function getARequestHandler() { + exists(Flask::FlaskViewClass cls | + cls.getADecorator().getAFlowNode() = node and + result = cls.getARequestHandler() + ) + } + } +} diff --git a/python/ql/src/meta/alerts/RequestHandlers.ql b/python/ql/src/meta/alerts/RequestHandlers.ql new file mode 100644 index 000000000000..2f79a3f1b352 --- /dev/null +++ b/python/ql/src/meta/alerts/RequestHandlers.ql @@ -0,0 +1,23 @@ +/** + * @name Request Handlers + * @description HTTP Server Request Handlers + * @kind problem + * @problem.severity recommendation + * @id py/meta/alerts/request-handlers + * @tags meta + * @precision very-low + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.Concepts +private import meta.MetaMetrics + +from HTTP::Server::RequestHandler requestHandler, string title +where + not requestHandler.getLocation().getFile() instanceof IgnoredFile and + if requestHandler.isMethod() + then + title = "Method " + requestHandler.getScope().(Class).getName() + "." + requestHandler.getName() + else title = requestHandler.toString() +select requestHandler, "RequestHandler: " + title diff --git a/python/ql/test/library-tests/frameworks/flask_admin/ConceptsTest.expected b/python/ql/test/library-tests/frameworks/flask_admin/ConceptsTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/library-tests/frameworks/flask_admin/ConceptsTest.ql b/python/ql/test/library-tests/frameworks/flask_admin/ConceptsTest.ql new file mode 100644 index 000000000000..1e2c1fab3eeb --- /dev/null +++ b/python/ql/test/library-tests/frameworks/flask_admin/ConceptsTest.ql @@ -0,0 +1,12 @@ +import python +import experimental.meta.ConceptsTest + +class DedicatedResponseTest extends HttpServerHttpResponseTest { + DedicatedResponseTest() { file.getShortName() = "response_test.py" } +} + +class OtherResponseTest extends HttpServerHttpResponseTest { + OtherResponseTest() { not this instanceof DedicatedResponseTest } + + override string getARelevantTag() { result = "HttpResponse" } +} diff --git a/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.expected b/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.expected new file mode 100644 index 000000000000..79d760d87f42 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.expected @@ -0,0 +1,3 @@ +argumentToEnsureNotTaintedNotMarkedAsSpurious +untaintedArgumentToEnsureTaintedNotMarkedAsMissing +failures diff --git a/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.ql b/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.ql new file mode 100644 index 000000000000..027ad8667be6 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/flask_admin/InlineTaintTest.ql @@ -0,0 +1 @@ +import experimental.meta.InlineTaintTest diff --git a/python/ql/test/library-tests/frameworks/flask_admin/test.py b/python/ql/test/library-tests/frameworks/flask_admin/test.py new file mode 100644 index 000000000000..9134cde4acc1 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/flask_admin/test.py @@ -0,0 +1,58 @@ +from flask import Flask, redirect +from flask.views import MethodView +import flask_admin + +ensure_tainted = ensure_not_tainted = print + + +app = Flask(__name__) + +# unknown at least for our current analysis +foo = "'/foo'" +UNKNOWN_ROUTE = eval(foo) # $ getCode=foo + + +class ExampleClass(flask_admin.BaseView): + @flask_admin.expose('/') # $ routeSetup="/" + def foo(self): # $ requestHandler + return "foo" # $ HttpResponse + + @flask_admin.expose(url='/bar/') # $ routeSetup="/bar/" + def bar(self, arg): # $ requestHandler routedParameter=arg + ensure_tainted(arg) # $ tainted + return "bar: " + arg # $ HttpResponse + + @flask_admin.expose_plugview("/flask-class") # $ routeSetup="/flask-class" + @flask_admin.expose_plugview(url="/flask-class/") # $ routeSetup="/flask-class/" + class Nested(MethodView): + def get(self, cls, arg="default"): # $ requestHandler routedParameter=arg + assert isinstance(cls, ExampleClass) + ensure_tainted(arg) # $ tainted + ensure_not_tainted(cls) + return "GET: " + arg # $ HttpResponse + + def post(self, cls, arg): # $ requestHandler routedParameter=arg + assert isinstance(cls, ExampleClass) + ensure_tainted(arg) # $ tainted + ensure_not_tainted(cls) + return "POST: " + arg # $ HttpResponse + + @flask_admin.expose_plugview(UNKNOWN_ROUTE) # $ routeSetup + class WithUnknownRoute(MethodView): + def get(self, cls, maybeRouted): # $ requestHandler routedParameter=maybeRouted + ensure_tainted(maybeRouted) # $ tainted + ensure_not_tainted(cls) + return "ok" # $ HttpResponse + + +@app.route('/') # $ routeSetup="/" +def index(): # $ requestHandler + return redirect('/admin') # $ HttpRedirectResponse HttpResponse redirectLocation='/admin' + + +if __name__ == "__main__": + admin = flask_admin.Admin(app, name="Some Admin Interface") + admin.add_view(ExampleClass()) + + print(app.url_map) + app.run(debug=True)