From 6f89b3f3d967f4fd39389f3d9beff73bd958f609 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Sat, 20 Mar 2021 00:01:40 +0100 Subject: [PATCH 01/16] Init Header Injection query --- .../Security/CWE-113/HeaderInjection.ql | 89 +++++++++++++++++++ .../Security/CWE-113/HeaderInjection.qlref | 0 .../Security/CWE-113/tests/django_bad.py | 15 ++++ .../Security/CWE-113/tests/flask_bad.py | 47 ++++++++++ 4 files changed, 151 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql create mode 100644 python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref create mode 100644 python/ql/src/experimental/Security/CWE-113/tests/django_bad.py create mode 100644 python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql new file mode 100644 index 00000000000..d03395f571d --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql @@ -0,0 +1,89 @@ +/** + * @name HTTP Header Injection + * @description User input should not be used in HTTP headers without first being escaped, + * otherwise a malicious user may be able to inject a value that could manipulate the response. + * @kind path-problem + * @problem.severity error + * @id python/header-injection + * @tags security + * external/cwe/cwe-113 + * external/cwe/cwe-079 + */ + +// determine precision above +import python +import semmle.python.dataflow.new.RemoteFlowSources +import semmle.python.dataflow.new.DataFlow +import semmle.python.dataflow.new.TaintTracking +import semmle.python.ApiGraphs +import DataFlow::PathGraph +import semmle.python.frameworks.Flask + +class WerkzeugHeader extends DataFlow::Node { + WerkzeugHeader() { + exists(DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead addMethod | + headerInstance = + API::moduleImport("werkzeug").getMember("datastructures").getMember("Headers").getACall() and + addMethod.getAttributeName() = "add" and + addMethod.getObject().getALocalSource() = headerInstance and + this = addMethod.(DataFlow::CallCfgNode).getArg(1) + ) + } +} + +class FlaskHeader extends DataFlow::Node { + FlaskHeader() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + AssignStmt sinkDeclaration + | + headerInstance = API::moduleImport("flask").getMember("Response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and + this.asExpr() = sinkDeclaration.getValue() + ) + } +} + +class FlaskMakeResponse extends DataFlow::Node { + FlaskMakeResponse() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + AssignStmt sinkDeclaration + | + headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + ( + sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and + this.asExpr() = sinkDeclaration.getValue() + ) + //or + //extendMethod.getAttributeName() = "extend" and + //extendMethod.getObject().getALocalSource() = responseMethod and + //this = extendMethod.(DataFlow::CallCfgNode).getArg(0) + ) + } +} + +class HeaderInjectionSink extends DataFlow::Node { + HeaderInjectionSink() { + this instanceof WerkzeugHeader or + this instanceof FlaskHeader or + this instanceof FlaskMakeResponse + } +} + +class HeaderInjectionFlowConfig extends TaintTracking::Configuration { + HeaderInjectionFlowConfig() { this = "HeaderInjectionFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderInjectionSink } +} + +from HeaderInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "$@ header is constructed from a $@.", sink.getNode(), "This", + source.getNode(), "user-provided value" diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/src/experimental/Security/CWE-113/tests/django_bad.py b/python/ql/src/experimental/Security/CWE-113/tests/django_bad.py new file mode 100644 index 00000000000..b97d9137f46 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-113/tests/django_bad.py @@ -0,0 +1,15 @@ +import django.http + + +def django_setitem(request): + rfs_header = request.GET.get("rfs_header") + response = django.http.HttpResponse() + response.__setitem__('HeaderName', rfs_header) + return response + + +def django_response(request): + rfs_header = request.GET.get("rfs_header") + response = django.http.HttpResponse() + response['HeaderName'] = rfs_header + return response diff --git a/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py b/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py new file mode 100644 index 00000000000..1f011fe8802 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py @@ -0,0 +1,47 @@ +from flask import Response, request, Flask +from werkzeug.datastructures import Headers + +app = Flask(__name__) + + +@app.route('/werkzeug_headers') +def werkzeug_headers(): + rfs_header = request.args["rfs_header"] + response = Response() + headers = Headers() + headers.add("HeaderName", rfs_header) + response.headers = headers + return response + + +@app.route("/flask_Response") +def flask_Response(): + rfs_header = request.args["rfs_header"] + response = Response() + response.headers['HeaderName'] = rfs_header + return response + + +@app.route("/flask_make_response") +def flask_make_response(): + rfs_header = request.args["rfs_header"] + resp = make_response("hello") + resp.headers['HeaderName'] = rfs_header + return resp + + +@app.route("/flask_make_response_extend") +def flask_make_response_extend(): + rfs_header = request.args["rfs_header"] + resp = make_response("hello") + resp.headers.extend( + {'HeaderName': request.args["rfs_header"]}) + return resp + + +@app.route("/Response_arg") +def Response_arg(): + return Response(headers={'HeaderName': request.args["rfs_header"]}) + +# if __name__ == "__main__": +# app.run(debug=True) From 46c5cb1136c619a5a59744462ce63ad241e607a7 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Thu, 1 Apr 2021 14:21:45 +0200 Subject: [PATCH 02/16] Polish WerkzeugHeaderCall --- .../Security/CWE-113/HeaderInjection.ql | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql index d03395f571d..52180bd1372 100644 --- a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql @@ -17,18 +17,18 @@ import semmle.python.dataflow.new.DataFlow import semmle.python.dataflow.new.TaintTracking import semmle.python.ApiGraphs import DataFlow::PathGraph -import semmle.python.frameworks.Flask -class WerkzeugHeader extends DataFlow::Node { - WerkzeugHeader() { - exists(DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead addMethod | - headerInstance = +class WerkzeugHeaderCall extends DataFlow::CallCfgNode { + WerkzeugHeaderCall() { + exists(DataFlow::AttrRead addMethod | + this.getFunction() = addMethod and + addMethod.getObject().getALocalSource() = API::moduleImport("werkzeug").getMember("datastructures").getMember("Headers").getACall() and - addMethod.getAttributeName() = "add" and - addMethod.getObject().getALocalSource() = headerInstance and - this = addMethod.(DataFlow::CallCfgNode).getArg(1) + addMethod.getAttributeName() = "add" ) } + + DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } } class FlaskHeader extends DataFlow::Node { From 3be916e82b16e744146fdb5a7659c796fc2a543f Mon Sep 17 00:00:00 2001 From: jorgectf Date: Thu, 1 Apr 2021 15:56:03 +0200 Subject: [PATCH 03/16] Polish FlaskHeaderCall --- .../Security/CWE-113/HeaderInjection.ql | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql index 52180bd1372..e3f16eb80f5 100644 --- a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql @@ -31,8 +31,10 @@ class WerkzeugHeaderCall extends DataFlow::CallCfgNode { DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } } -class FlaskHeader extends DataFlow::Node { - FlaskHeader() { +class FlaskHeaderCall extends DataFlow::CallCfgNode { + DataFlow::Node headerInputNode; + + FlaskHeaderCall() { exists( DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, AssignStmt sinkDeclaration @@ -41,9 +43,12 @@ class FlaskHeader extends DataFlow::Node { responseMethod.getAttributeName() = "headers" and responseMethod.getObject().getALocalSource() = headerInstance and sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - this.asExpr() = sinkDeclaration.getValue() + headerInputNode.asExpr() = sinkDeclaration.getValue() and + this.getFunction() = responseMethod ) } + + DataFlow::Node getHeaderInputNode() { result = headerInputNode } } class FlaskMakeResponse extends DataFlow::Node { @@ -69,8 +74,8 @@ class FlaskMakeResponse extends DataFlow::Node { class HeaderInjectionSink extends DataFlow::Node { HeaderInjectionSink() { - this instanceof WerkzeugHeader or - this instanceof FlaskHeader or + this instanceof WerkzeugHeaderCall or + this instanceof FlaskHeaderCall or this instanceof FlaskMakeResponse } } From bd894ae8b3c254799deb2ae48b9e51e30fc66364 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Sun, 4 Apr 2021 01:16:28 +0200 Subject: [PATCH 04/16] Fix flask test --- .../ql/src/experimental/Security/CWE-113/tests/flask_bad.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py b/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py index 1f011fe8802..6f2968efb55 100644 --- a/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py +++ b/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py @@ -1,4 +1,4 @@ -from flask import Response, request, Flask +from flask import Response, request, Flask, make_response from werkzeug.datastructures import Headers app = Flask(__name__) @@ -35,7 +35,7 @@ def flask_make_response_extend(): rfs_header = request.args["rfs_header"] resp = make_response("hello") resp.headers.extend( - {'HeaderName': request.args["rfs_header"]}) + {'HeaderName': rfs_header}) return resp From 6158dd6bce85529e3661acd432c1e3abdbce51dd Mon Sep 17 00:00:00 2001 From: jorgectf Date: Sun, 4 Apr 2021 01:16:38 +0200 Subject: [PATCH 05/16] Finish Sinks --- .../Security/CWE-113/HeaderInjection.ql | 99 ++++++++++++++++--- 1 file changed, 84 insertions(+), 15 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql index e3f16eb80f5..c5d2984c150 100644 --- a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql @@ -31,7 +31,7 @@ class WerkzeugHeaderCall extends DataFlow::CallCfgNode { DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } } -class FlaskHeaderCall extends DataFlow::CallCfgNode { +class FlaskHeaderCall extends DataFlow::Node { DataFlow::Node headerInputNode; FlaskHeaderCall() { @@ -44,15 +44,17 @@ class FlaskHeaderCall extends DataFlow::CallCfgNode { responseMethod.getObject().getALocalSource() = headerInstance and sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and headerInputNode.asExpr() = sinkDeclaration.getValue() and - this.getFunction() = responseMethod + this.asExpr() = sinkDeclaration.getATarget() ) } DataFlow::Node getHeaderInputNode() { result = headerInputNode } } -class FlaskMakeResponse extends DataFlow::Node { - FlaskMakeResponse() { +class FlaskMakeResponseCall extends DataFlow::Node { + DataFlow::Node headerInputNode; + + FlaskMakeResponseCall() { exists( DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, AssignStmt sinkDeclaration @@ -60,23 +62,90 @@ class FlaskMakeResponse extends DataFlow::Node { headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and responseMethod.getAttributeName() = "headers" and responseMethod.getObject().getALocalSource() = headerInstance and - ( - sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - this.asExpr() = sinkDeclaration.getValue() - ) - //or - //extendMethod.getAttributeName() = "extend" and - //extendMethod.getObject().getALocalSource() = responseMethod and - //this = extendMethod.(DataFlow::CallCfgNode).getArg(0) + sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and + this.asExpr() = sinkDeclaration.getATarget() and + headerInputNode.asExpr() = sinkDeclaration.getValue() ) } + + DataFlow::Node getHeaderInputNode() { result = headerInputNode } +} + +class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode { + DataFlow::Node headerInputNode; + + FlaskMakeResponseExtendCall() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + DataFlow::AttrRead extendMethod + | + headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + extendMethod.getAttributeName() = "extend" and + extendMethod.getObject().getALocalSource() = responseMethod and + this.getFunction() = extendMethod and + headerInputNode = this.getArg(0) + ) + } + + DataFlow::Node getHeaderInputNode() { result = headerInputNode } +} + +class FlaskResponseArg extends DataFlow::CallCfgNode { + DataFlow::Node headerInputNode; + + FlaskResponseArg() { + this = API::moduleImport("flask").getMember("Response").getACall() and + headerInputNode = this.getArgByName("headers") + } + + DataFlow::Node getHeaderInputNode() { result = headerInputNode } +} + +class DjangoResponseSetItemCall extends DataFlow::CallCfgNode { + DjangoResponseSetItemCall() { + exists(DataFlow::AttrRead setItemMethod | + this.getFunction() = setItemMethod and + setItemMethod.getObject().getALocalSource() = + API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and + setItemMethod.getAttributeName() = "__setitem__" + ) + } + + DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } +} + +class DjangoResponseAssignCall extends DataFlow::Node { + DataFlow::Node headerInputNode; + + DjangoResponseAssignCall() { + exists( + DataFlow::CallCfgNode headerInstance, Subscript responseMethod, DataFlow::Node responseToNode, + AssignStmt sinkDeclaration + | + headerInstance = + API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and + responseMethod.getValue() = responseToNode.asExpr() and + responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and + sinkDeclaration.getATarget() = responseMethod and + this.asExpr() = sinkDeclaration.getATarget() and + headerInputNode.asExpr() = sinkDeclaration.getValue() + ) + } + + DataFlow::Node getHeaderInputNode() { result = headerInputNode } } class HeaderInjectionSink extends DataFlow::Node { HeaderInjectionSink() { - this instanceof WerkzeugHeaderCall or - this instanceof FlaskHeaderCall or - this instanceof FlaskMakeResponse + this = any(WerkzeugHeaderCall a).getHeaderInputNode() or + this = any(FlaskHeaderCall a).getHeaderInputNode() or + this = any(FlaskMakeResponseCall a).getHeaderInputNode() or + this = any(FlaskMakeResponseExtendCall a).getHeaderInputNode() or + this = any(FlaskResponseArg a).getHeaderInputNode() or + this = any(DjangoResponseSetItemCall a).getHeaderInputNode() or + this = any(DjangoResponseAssignCall a).getHeaderInputNode() } } From b0c498629a39b2a02815ab9495193e1bd475f155 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:01:29 +0200 Subject: [PATCH 06/16] Init restructuring --- .../python/security/injection/HTTPHeaders.qll | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll diff --git a/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll new file mode 100644 index 00000000000..1215710c1aa --- /dev/null +++ b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll @@ -0,0 +1,15 @@ +import python +import experimental.semmle.python.Concepts +import semmle.python.dataflow.new.DataFlow +import semmle.python.dataflow.new.TaintTracking +import semmle.python.dataflow.new.RemoteFlowSources + +class HeaderInjectionFlowConfig extends TaintTracking::Configuration { + HeaderInjectionFlowConfig() { this = "HeaderInjectionFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { + sink = any(HeaderDeclaration headerDeclaration).getHeaderInputNode() + } +} From ce3fb6be21fc84248b387527f62367616a1fc79d Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:26:16 +0200 Subject: [PATCH 07/16] Improve qhelp --- .../Security/CWE-113/HeaderInjection.qhelp | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-113/HeaderInjection.qhelp diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qhelp b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qhelp new file mode 100644 index 00000000000..33337294b9a --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qhelp @@ -0,0 +1,26 @@ + + + +

If an HTTP Header is built using string concatenation or string formatting, and the +components of the concatenation include user input, a user +is likely to be able to manipulate the response.

+
+ + +

User input should not be included in an HTTP Header.

+
+ + +

In the following example, the code appends a user-provided value into a header.

+ + +
+ + +
  • OWASP: HTTP Response Splitting.
  • +
  • Python Security: HTTP header injection.
  • +
  • SonarSource: RSPEC-5167.
  • +
    +
    From 789c5857fa2962a31e3271594af2971519eb072a Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:26:28 +0200 Subject: [PATCH 08/16] Create qhelp example --- .../experimental/Security/CWE-113/header_injection.py | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-113/header_injection.py diff --git a/python/ql/src/experimental/Security/CWE-113/header_injection.py b/python/ql/src/experimental/Security/CWE-113/header_injection.py new file mode 100644 index 00000000000..117383710e3 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-113/header_injection.py @@ -0,0 +1,9 @@ +from flask import Response, request, Flask, make_response + + +@app.route("/flask_Response") +def flask_Response(): + rfs_header = request.args["rfs_header"] + response = Response() + response.headers['HeaderName'] = rfs_header + return response From e9c457455252654ce4fdade7edaf7e4047e2723b Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:26:53 +0200 Subject: [PATCH 09/16] Apply structure --- .../Security/CWE-113/HeaderInjection.ql | 154 +----------------- .../Security/CWE-113/HeaderInjection.qlref | 0 .../experimental/semmle/python/Concepts.qll | 14 ++ .../semmle/python/frameworks/Stdlib.qll | 130 +++++++++++++++ .../Security/CWE-113}/django_bad.py | 0 .../Security/CWE-113}/flask_bad.py | 0 6 files changed, 150 insertions(+), 148 deletions(-) delete mode 100644 python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref rename python/ql/{src/experimental/Security/CWE-113/tests => test/experimental/query-tests/Security/CWE-113}/django_bad.py (100%) rename python/ql/{src/experimental/Security/CWE-113/tests => test/experimental/query-tests/Security/CWE-113}/flask_bad.py (100%) diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql index c5d2984c150..3cb4a20d5de 100644 --- a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql +++ b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql @@ -1,10 +1,10 @@ /** * @name HTTP Header Injection - * @description User input should not be used in HTTP headers without first being escaped, - * otherwise a malicious user may be able to inject a value that could manipulate the response. + * @description User input should not be used in HTTP headers, otherwise a malicious user + * may be able to inject a value that could manipulate the response. * @kind path-problem * @problem.severity error - * @id python/header-injection + * @id py/header-injection * @tags security * external/cwe/cwe-113 * external/cwe/cwe-079 @@ -12,152 +12,10 @@ // determine precision above import python -import semmle.python.dataflow.new.RemoteFlowSources -import semmle.python.dataflow.new.DataFlow -import semmle.python.dataflow.new.TaintTracking -import semmle.python.ApiGraphs +import experimental.semmle.python.security.injection.HTTPHeaders import DataFlow::PathGraph -class WerkzeugHeaderCall extends DataFlow::CallCfgNode { - WerkzeugHeaderCall() { - exists(DataFlow::AttrRead addMethod | - this.getFunction() = addMethod and - addMethod.getObject().getALocalSource() = - API::moduleImport("werkzeug").getMember("datastructures").getMember("Headers").getACall() and - addMethod.getAttributeName() = "add" - ) - } - - DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } -} - -class FlaskHeaderCall extends DataFlow::Node { - DataFlow::Node headerInputNode; - - FlaskHeaderCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - AssignStmt sinkDeclaration - | - headerInstance = API::moduleImport("flask").getMember("Response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - headerInputNode.asExpr() = sinkDeclaration.getValue() and - this.asExpr() = sinkDeclaration.getATarget() - ) - } - - DataFlow::Node getHeaderInputNode() { result = headerInputNode } -} - -class FlaskMakeResponseCall extends DataFlow::Node { - DataFlow::Node headerInputNode; - - FlaskMakeResponseCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - AssignStmt sinkDeclaration - | - headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - this.asExpr() = sinkDeclaration.getATarget() and - headerInputNode.asExpr() = sinkDeclaration.getValue() - ) - } - - DataFlow::Node getHeaderInputNode() { result = headerInputNode } -} - -class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode { - DataFlow::Node headerInputNode; - - FlaskMakeResponseExtendCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - DataFlow::AttrRead extendMethod - | - headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - extendMethod.getAttributeName() = "extend" and - extendMethod.getObject().getALocalSource() = responseMethod and - this.getFunction() = extendMethod and - headerInputNode = this.getArg(0) - ) - } - - DataFlow::Node getHeaderInputNode() { result = headerInputNode } -} - -class FlaskResponseArg extends DataFlow::CallCfgNode { - DataFlow::Node headerInputNode; - - FlaskResponseArg() { - this = API::moduleImport("flask").getMember("Response").getACall() and - headerInputNode = this.getArgByName("headers") - } - - DataFlow::Node getHeaderInputNode() { result = headerInputNode } -} - -class DjangoResponseSetItemCall extends DataFlow::CallCfgNode { - DjangoResponseSetItemCall() { - exists(DataFlow::AttrRead setItemMethod | - this.getFunction() = setItemMethod and - setItemMethod.getObject().getALocalSource() = - API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and - setItemMethod.getAttributeName() = "__setitem__" - ) - } - - DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } -} - -class DjangoResponseAssignCall extends DataFlow::Node { - DataFlow::Node headerInputNode; - - DjangoResponseAssignCall() { - exists( - DataFlow::CallCfgNode headerInstance, Subscript responseMethod, DataFlow::Node responseToNode, - AssignStmt sinkDeclaration - | - headerInstance = - API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and - responseMethod.getValue() = responseToNode.asExpr() and - responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and - sinkDeclaration.getATarget() = responseMethod and - this.asExpr() = sinkDeclaration.getATarget() and - headerInputNode.asExpr() = sinkDeclaration.getValue() - ) - } - - DataFlow::Node getHeaderInputNode() { result = headerInputNode } -} - -class HeaderInjectionSink extends DataFlow::Node { - HeaderInjectionSink() { - this = any(WerkzeugHeaderCall a).getHeaderInputNode() or - this = any(FlaskHeaderCall a).getHeaderInputNode() or - this = any(FlaskMakeResponseCall a).getHeaderInputNode() or - this = any(FlaskMakeResponseExtendCall a).getHeaderInputNode() or - this = any(FlaskResponseArg a).getHeaderInputNode() or - this = any(DjangoResponseSetItemCall a).getHeaderInputNode() or - this = any(DjangoResponseAssignCall a).getHeaderInputNode() - } -} - -class HeaderInjectionFlowConfig extends TaintTracking::Configuration { - HeaderInjectionFlowConfig() { this = "HeaderInjectionFlowConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderInjectionSink } -} - from HeaderInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink where config.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "$@ header is constructed from a $@.", sink.getNode(), "This", - source.getNode(), "user-provided value" +select sink.getNode(), source, sink, "$@ HTTP header is constructed from a $@.", sink.getNode(), + "This", source.getNode(), "user-provided value" diff --git a/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref b/python/ql/src/experimental/Security/CWE-113/HeaderInjection.qlref deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll index 904b7967ee8..530b8d20ec6 100644 --- a/python/ql/src/experimental/semmle/python/Concepts.qll +++ b/python/ql/src/experimental/semmle/python/Concepts.qll @@ -13,3 +13,17 @@ private import semmle.python.dataflow.new.DataFlow private import semmle.python.dataflow.new.RemoteFlowSources private import semmle.python.dataflow.new.TaintTracking private import experimental.semmle.python.Frameworks + +module HeaderDeclaration { + abstract class Range extends DataFlow::Node { + abstract DataFlow::Node getHeaderInputNode(); + } +} + +class HeaderDeclaration extends DataFlow::Node { + HeaderDeclaration::Range range; + + HeaderDeclaration() { this = range } + + DataFlow::Node getHeaderInputNode() { result = range.getHeaderInputNode() } +} diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll index 420caf0d73b..636a0621bd4 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll @@ -9,3 +9,133 @@ private import semmle.python.dataflow.new.TaintTracking private import semmle.python.dataflow.new.RemoteFlowSources private import experimental.semmle.python.Concepts private import semmle.python.ApiGraphs + +private module Headers { + private module Werkzeug { + class WerkzeugHeaderCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + WerkzeugHeaderCall() { + exists(DataFlow::AttrRead addMethod | + this.getFunction() = addMethod and + addMethod.getObject().getALocalSource() = + API::moduleImport("werkzeug") + .getMember("datastructures") + .getMember("Headers") + .getACall() and + addMethod.getAttributeName() = "add" + ) + } + + override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } + } + } + + private module Flask { + class FlaskHeaderCall extends DataFlow::Node, HeaderDeclaration::Range { + DataFlow::Node headerInputNode; + + FlaskHeaderCall() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + AssignStmt sinkDeclaration + | + headerInstance = API::moduleImport("flask").getMember("Response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and + headerInputNode.asExpr() = sinkDeclaration.getValue() and + this.asExpr() = sinkDeclaration.getATarget() + ) + } + + override DataFlow::Node getHeaderInputNode() { result = headerInputNode } + } + + class FlaskMakeResponseCall extends DataFlow::Node, HeaderDeclaration::Range { + DataFlow::Node headerInputNode; + + FlaskMakeResponseCall() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + AssignStmt sinkDeclaration + | + headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and + this.asExpr() = sinkDeclaration.getATarget() and + headerInputNode.asExpr() = sinkDeclaration.getValue() + ) + } + + override DataFlow::Node getHeaderInputNode() { result = headerInputNode } + } + + class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + DataFlow::Node headerInputNode; + + FlaskMakeResponseExtendCall() { + exists( + DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, + DataFlow::AttrRead extendMethod + | + headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and + responseMethod.getAttributeName() = "headers" and + responseMethod.getObject().getALocalSource() = headerInstance and + extendMethod.getAttributeName() = "extend" and + extendMethod.getObject().getALocalSource() = responseMethod and + this.getFunction() = extendMethod and + headerInputNode = this.getArg(0) + ) + } + + override DataFlow::Node getHeaderInputNode() { result = headerInputNode } + } + + class FlaskResponseArg extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + DataFlow::Node headerInputNode; + + FlaskResponseArg() { + this = API::moduleImport("flask").getMember("Response").getACall() and + headerInputNode = this.getArgByName("headers") + } + + override DataFlow::Node getHeaderInputNode() { result = headerInputNode } + } + + class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + DjangoResponseSetItemCall() { + exists(DataFlow::AttrRead setItemMethod | + this.getFunction() = setItemMethod and + setItemMethod.getObject().getALocalSource() = + API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and + setItemMethod.getAttributeName() = "__setitem__" + ) + } + + override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } + } + } + + private module Django { + class DjangoResponseAssignCall extends DataFlow::Node, HeaderDeclaration::Range { + DataFlow::Node headerInputNode; + + DjangoResponseAssignCall() { + exists( + DataFlow::CallCfgNode headerInstance, Subscript responseMethod, + DataFlow::Node responseToNode, AssignStmt sinkDeclaration + | + headerInstance = + API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and + responseMethod.getValue() = responseToNode.asExpr() and + responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and + sinkDeclaration.getATarget() = responseMethod and + this.asExpr() = sinkDeclaration.getATarget() and + headerInputNode.asExpr() = sinkDeclaration.getValue() + ) + } + + override DataFlow::Node getHeaderInputNode() { result = headerInputNode } + } + } +} diff --git a/python/ql/src/experimental/Security/CWE-113/tests/django_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-113/django_bad.py similarity index 100% rename from python/ql/src/experimental/Security/CWE-113/tests/django_bad.py rename to python/ql/test/experimental/query-tests/Security/CWE-113/django_bad.py diff --git a/python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-113/flask_bad.py similarity index 100% rename from python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py rename to python/ql/test/experimental/query-tests/Security/CWE-113/flask_bad.py From 632dc61d5e6778fe9aa6767f257e0942b3be0010 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:28:22 +0200 Subject: [PATCH 10/16] Create qlref --- .../query-tests/Security/CWE-113/HeaderInjection.qlref | 1 + 1 file changed, 1 insertion(+) create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.qlref diff --git a/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.qlref b/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.qlref new file mode 100644 index 00000000000..915175a7b6a --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-113/HeaderInjection.ql From f02c2855adc8e017f9c1d1a60aff3bc8124e27f0 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 9 Apr 2021 01:28:38 +0200 Subject: [PATCH 11/16] Generate .expected --- .../Security/CWE-113/HeaderInjection.expected | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.expected diff --git a/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.expected b/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.expected new file mode 100644 index 00000000000..9e0eff704b6 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-113/HeaderInjection.expected @@ -0,0 +1,43 @@ +edges +| flask_bad.py:9:18:9:24 | ControlFlowNode for request | flask_bad.py:9:18:9:29 | ControlFlowNode for Attribute | +| flask_bad.py:9:18:9:29 | ControlFlowNode for Attribute | flask_bad.py:9:18:9:43 | ControlFlowNode for Subscript | +| flask_bad.py:9:18:9:43 | ControlFlowNode for Subscript | flask_bad.py:12:31:12:40 | ControlFlowNode for rfs_header | +| flask_bad.py:19:18:19:24 | ControlFlowNode for request | flask_bad.py:19:18:19:29 | ControlFlowNode for Attribute | +| flask_bad.py:19:18:19:29 | ControlFlowNode for Attribute | flask_bad.py:19:18:19:43 | ControlFlowNode for Subscript | +| flask_bad.py:19:18:19:43 | ControlFlowNode for Subscript | flask_bad.py:21:38:21:47 | ControlFlowNode for rfs_header | +| flask_bad.py:27:18:27:24 | ControlFlowNode for request | flask_bad.py:27:18:27:29 | ControlFlowNode for Attribute | +| flask_bad.py:27:18:27:29 | ControlFlowNode for Attribute | flask_bad.py:27:18:27:43 | ControlFlowNode for Subscript | +| flask_bad.py:27:18:27:43 | ControlFlowNode for Subscript | flask_bad.py:29:34:29:43 | ControlFlowNode for rfs_header | +| flask_bad.py:35:18:35:24 | ControlFlowNode for request | flask_bad.py:35:18:35:29 | ControlFlowNode for Attribute | +| flask_bad.py:35:18:35:29 | ControlFlowNode for Attribute | flask_bad.py:35:18:35:43 | ControlFlowNode for Subscript | +| flask_bad.py:35:18:35:43 | ControlFlowNode for Subscript | flask_bad.py:38:9:38:34 | ControlFlowNode for Dict | +| flask_bad.py:44:44:44:50 | ControlFlowNode for request | flask_bad.py:44:44:44:55 | ControlFlowNode for Attribute | +| flask_bad.py:44:44:44:55 | ControlFlowNode for Attribute | flask_bad.py:44:44:44:69 | ControlFlowNode for Subscript | +| flask_bad.py:44:44:44:69 | ControlFlowNode for Subscript | flask_bad.py:44:29:44:70 | ControlFlowNode for Dict | +nodes +| flask_bad.py:9:18:9:24 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| flask_bad.py:9:18:9:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| flask_bad.py:9:18:9:43 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | +| flask_bad.py:12:31:12:40 | ControlFlowNode for rfs_header | semmle.label | ControlFlowNode for rfs_header | +| flask_bad.py:19:18:19:24 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| flask_bad.py:19:18:19:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| flask_bad.py:19:18:19:43 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | +| flask_bad.py:21:38:21:47 | ControlFlowNode for rfs_header | semmle.label | ControlFlowNode for rfs_header | +| flask_bad.py:27:18:27:24 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| flask_bad.py:27:18:27:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| flask_bad.py:27:18:27:43 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | +| flask_bad.py:29:34:29:43 | ControlFlowNode for rfs_header | semmle.label | ControlFlowNode for rfs_header | +| flask_bad.py:35:18:35:24 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| flask_bad.py:35:18:35:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| flask_bad.py:35:18:35:43 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | +| flask_bad.py:38:9:38:34 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict | +| flask_bad.py:44:29:44:70 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict | +| flask_bad.py:44:44:44:50 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| flask_bad.py:44:44:44:55 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| flask_bad.py:44:44:44:69 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript | +#select +| flask_bad.py:12:31:12:40 | ControlFlowNode for rfs_header | flask_bad.py:9:18:9:24 | ControlFlowNode for request | flask_bad.py:12:31:12:40 | ControlFlowNode for rfs_header | $@ HTTP header is constructed from a $@. | flask_bad.py:12:31:12:40 | ControlFlowNode for rfs_header | This | flask_bad.py:9:18:9:24 | ControlFlowNode for request | user-provided value | +| flask_bad.py:21:38:21:47 | ControlFlowNode for rfs_header | flask_bad.py:19:18:19:24 | ControlFlowNode for request | flask_bad.py:21:38:21:47 | ControlFlowNode for rfs_header | $@ HTTP header is constructed from a $@. | flask_bad.py:21:38:21:47 | ControlFlowNode for rfs_header | This | flask_bad.py:19:18:19:24 | ControlFlowNode for request | user-provided value | +| flask_bad.py:29:34:29:43 | ControlFlowNode for rfs_header | flask_bad.py:27:18:27:24 | ControlFlowNode for request | flask_bad.py:29:34:29:43 | ControlFlowNode for rfs_header | $@ HTTP header is constructed from a $@. | flask_bad.py:29:34:29:43 | ControlFlowNode for rfs_header | This | flask_bad.py:27:18:27:24 | ControlFlowNode for request | user-provided value | +| flask_bad.py:38:9:38:34 | ControlFlowNode for Dict | flask_bad.py:35:18:35:24 | ControlFlowNode for request | flask_bad.py:38:9:38:34 | ControlFlowNode for Dict | $@ HTTP header is constructed from a $@. | flask_bad.py:38:9:38:34 | ControlFlowNode for Dict | This | flask_bad.py:35:18:35:24 | ControlFlowNode for request | user-provided value | +| flask_bad.py:44:29:44:70 | ControlFlowNode for Dict | flask_bad.py:44:44:44:50 | ControlFlowNode for request | flask_bad.py:44:29:44:70 | ControlFlowNode for Dict | $@ HTTP header is constructed from a $@. | flask_bad.py:44:29:44:70 | ControlFlowNode for Dict | This | flask_bad.py:44:44:44:50 | ControlFlowNode for request | user-provided value | From 066504e79efaf953ab000bae3bec966193bba57f Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 18 Jun 2021 02:02:47 +0200 Subject: [PATCH 12/16] Checkout Stdlib.qll --- .../semmle/python/frameworks/Stdlib.qll | 130 ------------------ 1 file changed, 130 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll index 636a0621bd4..420caf0d73b 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll @@ -9,133 +9,3 @@ private import semmle.python.dataflow.new.TaintTracking private import semmle.python.dataflow.new.RemoteFlowSources private import experimental.semmle.python.Concepts private import semmle.python.ApiGraphs - -private module Headers { - private module Werkzeug { - class WerkzeugHeaderCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { - WerkzeugHeaderCall() { - exists(DataFlow::AttrRead addMethod | - this.getFunction() = addMethod and - addMethod.getObject().getALocalSource() = - API::moduleImport("werkzeug") - .getMember("datastructures") - .getMember("Headers") - .getACall() and - addMethod.getAttributeName() = "add" - ) - } - - override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } - } - } - - private module Flask { - class FlaskHeaderCall extends DataFlow::Node, HeaderDeclaration::Range { - DataFlow::Node headerInputNode; - - FlaskHeaderCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - AssignStmt sinkDeclaration - | - headerInstance = API::moduleImport("flask").getMember("Response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - headerInputNode.asExpr() = sinkDeclaration.getValue() and - this.asExpr() = sinkDeclaration.getATarget() - ) - } - - override DataFlow::Node getHeaderInputNode() { result = headerInputNode } - } - - class FlaskMakeResponseCall extends DataFlow::Node, HeaderDeclaration::Range { - DataFlow::Node headerInputNode; - - FlaskMakeResponseCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - AssignStmt sinkDeclaration - | - headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and - this.asExpr() = sinkDeclaration.getATarget() and - headerInputNode.asExpr() = sinkDeclaration.getValue() - ) - } - - override DataFlow::Node getHeaderInputNode() { result = headerInputNode } - } - - class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { - DataFlow::Node headerInputNode; - - FlaskMakeResponseExtendCall() { - exists( - DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod, - DataFlow::AttrRead extendMethod - | - headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and - responseMethod.getAttributeName() = "headers" and - responseMethod.getObject().getALocalSource() = headerInstance and - extendMethod.getAttributeName() = "extend" and - extendMethod.getObject().getALocalSource() = responseMethod and - this.getFunction() = extendMethod and - headerInputNode = this.getArg(0) - ) - } - - override DataFlow::Node getHeaderInputNode() { result = headerInputNode } - } - - class FlaskResponseArg extends DataFlow::CallCfgNode, HeaderDeclaration::Range { - DataFlow::Node headerInputNode; - - FlaskResponseArg() { - this = API::moduleImport("flask").getMember("Response").getACall() and - headerInputNode = this.getArgByName("headers") - } - - override DataFlow::Node getHeaderInputNode() { result = headerInputNode } - } - - class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { - DjangoResponseSetItemCall() { - exists(DataFlow::AttrRead setItemMethod | - this.getFunction() = setItemMethod and - setItemMethod.getObject().getALocalSource() = - API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and - setItemMethod.getAttributeName() = "__setitem__" - ) - } - - override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) } - } - } - - private module Django { - class DjangoResponseAssignCall extends DataFlow::Node, HeaderDeclaration::Range { - DataFlow::Node headerInputNode; - - DjangoResponseAssignCall() { - exists( - DataFlow::CallCfgNode headerInstance, Subscript responseMethod, - DataFlow::Node responseToNode, AssignStmt sinkDeclaration - | - headerInstance = - API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and - responseMethod.getValue() = responseToNode.asExpr() and - responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and - sinkDeclaration.getATarget() = responseMethod and - this.asExpr() = sinkDeclaration.getATarget() and - headerInputNode.asExpr() = sinkDeclaration.getValue() - ) - } - - override DataFlow::Node getHeaderInputNode() { result = headerInputNode } - } - } -} From 4963caf5067c0467e7a88f1b361dfda09c4b9822 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 18 Jun 2021 02:03:27 +0200 Subject: [PATCH 13/16] Rewrite frameworks modeling --- .../experimental/semmle/python/Frameworks.qll | 3 + .../semmle/python/frameworks/Django.qll | 78 +++++++++++++++++++ .../semmle/python/frameworks/Flask.qll | 74 ++++++++++++++++++ .../semmle/python/frameworks/Werkzeug.qll | 31 ++++++++ 4 files changed, 186 insertions(+) create mode 100644 python/ql/src/experimental/semmle/python/frameworks/Django.qll create mode 100644 python/ql/src/experimental/semmle/python/frameworks/Flask.qll create mode 100644 python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll diff --git a/python/ql/src/experimental/semmle/python/Frameworks.qll b/python/ql/src/experimental/semmle/python/Frameworks.qll index ca1dd04e57d..da57f56e8cc 100644 --- a/python/ql/src/experimental/semmle/python/Frameworks.qll +++ b/python/ql/src/experimental/semmle/python/Frameworks.qll @@ -3,3 +3,6 @@ */ private import experimental.semmle.python.frameworks.Stdlib +private import experimental.semmle.python.frameworks.Flask +private import experimental.semmle.python.frameworks.Django +private import experimental.semmle.python.frameworks.Werkzeug diff --git a/python/ql/src/experimental/semmle/python/frameworks/Django.qll b/python/ql/src/experimental/semmle/python/frameworks/Django.qll new file mode 100644 index 00000000000..3353881fff8 --- /dev/null +++ b/python/ql/src/experimental/semmle/python/frameworks/Django.qll @@ -0,0 +1,78 @@ +/** + * Provides classes modeling security-relevant aspects of the `django` PyPI package. + * See https://www.djangoproject.com/. + */ + +private import python +private import semmle.python.frameworks.Django +private import semmle.python.dataflow.new.DataFlow +private import experimental.semmle.python.Concepts +private import semmle.python.ApiGraphs + +private module PrivateDjango { + API::Node django() { result = API::moduleImport("django") } + + private module django { + API::Node http() { result = django().getMember("http") } + + module http { + API::Node response() { result = http().getMember("response") } + + module response { + module HttpResponse { + API::Node baseClassRef() { + result = response().getMember("HttpResponse").getReturn() + or + // Handle `django.http.HttpResponse` alias + result = http().getMember("HttpResponse").getReturn() + } + + /** Gets a reference to a header instance. */ + private DataFlow::LocalSourceNode headerInstance(DataFlow::TypeTracker t) { + t.start() and + ( + exists(SubscriptNode subscript | + subscript.getObject() = baseClassRef().getAUse().asCfgNode() and + result.asCfgNode() = subscript + ) + or + result.(DataFlow::AttrRead).getObject() = baseClassRef().getAUse() + ) + or + exists(DataFlow::TypeTracker t2 | result = headerInstance(t2).track(t2, t)) + } + + /** Gets a reference to a header instance use. */ + private DataFlow::Node headerInstance() { + headerInstance(DataFlow::TypeTracker::end()).flowsTo(result) + } + + /** Gets a reference to a header instance call with `__setitem__`. */ + private DataFlow::Node headerSetItemCall() { + result = headerInstance() and + result.(DataFlow::AttrRead).getAttributeName() = "__setitem__" + } + + class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + DjangoResponseSetItemCall() { this.getFunction() = headerSetItemCall() } + + override DataFlow::Node getHeaderInput() { result = this.getArg([0, 1]) } + } + + class DjangoResponseDefinition extends DataFlow::Node, HeaderDeclaration::Range { + DataFlow::Node headerInput; + + DjangoResponseDefinition() { + this.asCfgNode().(DefinitionNode) = headerInstance().asCfgNode() and + headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue() + } + + override DataFlow::Node getHeaderInput() { + result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()] + } + } + } + } + } + } +} diff --git a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll new file mode 100644 index 00000000000..4ed3ed58e3c --- /dev/null +++ b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll @@ -0,0 +1,74 @@ +/** + * Provides classes modeling security-relevant aspects of the `flask` PyPI package. + * See https://flask.palletsprojects.com/en/1.1.x/. + */ + +private import python +private import semmle.python.frameworks.Flask +private import semmle.python.dataflow.new.DataFlow +private import experimental.semmle.python.Concepts +private import semmle.python.ApiGraphs + +module ExperimentalFlask { + /** + * A reference to either `flask.make_response` function, or the `make_response` method on + * an instance of `flask.Flask`. This creates an instance of the `flask_response` + * class (class-attribute on a flask application), which by default is + * `flask.Response`. + * + * See + * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.make_response + * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.make_response + */ + private API::Node flaskMakeResponse() { + result in [ + API::moduleImport("flask").getMember("make_response"), + Flask::FlaskApp::instance().getMember("make_response") + ] + } + + /** Gets a reference to a header instance. */ + private DataFlow::LocalSourceNode headerInstance(DataFlow::TypeTracker t) { + t.start() and + result.(DataFlow::AttrRead).getObject().getALocalSource() = + [Flask::Response::classRef(), flaskMakeResponse()].getReturn().getAUse() + or + exists(DataFlow::TypeTracker t2 | result = headerInstance(t2).track(t2, t)) + } + + /** Gets a reference to a header instance use. */ + private DataFlow::Node headerInstance() { + headerInstance(DataFlow::TypeTracker::end()).flowsTo(result) + } + + /** Gets a reference to a header instance call/subscript */ + private DataFlow::Node headerInstanceCall() { + headerInstance() in [result.(DataFlow::AttrRead), result.(DataFlow::AttrRead).getObject()] or + headerInstance().asExpr() = result.asExpr().(Subscript).getObject() + } + + class FlaskHeaderDefinition extends DataFlow::Node, HeaderDeclaration::Range { + DataFlow::Node headerInput; + + FlaskHeaderDefinition() { + this.asCfgNode().(DefinitionNode) = headerInstanceCall().asCfgNode() and + headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue() + } + + override DataFlow::Node getHeaderInput() { + result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()] + } + } + + private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + FlaskMakeResponseExtend() { this.getFunction() = headerInstanceCall() } + + override DataFlow::Node getHeaderInput() { result = this.getArg(0) } + } + + private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + FlaskResponse() { this = Flask::Response::classRef().getACall() } + + override DataFlow::Node getHeaderInput() { result = this.getArgByName("headers") } + } +} diff --git a/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll b/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll new file mode 100644 index 00000000000..4c7e68f165a --- /dev/null +++ b/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll @@ -0,0 +1,31 @@ +/** + * Provides classes modeling security-relevant aspects of the `Werkzeug` PyPI package. + * See + * - https://pypi.org/project/Werkzeug/ + * - https://werkzeug.palletsprojects.com/en/1.0.x/#werkzeug + */ + +private import python +private import semmle.python.frameworks.Flask +private import semmle.python.dataflow.new.DataFlow +private import experimental.semmle.python.Concepts +private import semmle.python.ApiGraphs + +private module Werkzeug { + module datastructures { + module Headers { + class WerkzeugHeaderAddCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { + WerkzeugHeaderAddCall() { + this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = + API::moduleImport("werkzeug") + .getMember("datastructures") + .getMember("Headers") + .getACall() and + this.getFunction().(DataFlow::AttrRead).getAttributeName() = "add" + } + + override DataFlow::Node getHeaderInput() { result = this.getArg(_) } + } + } + } +} From dcb1da338b1a8d4c378b1ac88d04c5e0dce970f9 Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 18 Jun 2021 02:03:56 +0200 Subject: [PATCH 14/16] Extend documentation --- .../experimental/semmle/python/Concepts.qll | 23 +++++++++++++++++-- .../python/security/injection/HTTPHeaders.qll | 5 +++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll index 530b8d20ec6..2036a6a671e 100644 --- a/python/ql/src/experimental/semmle/python/Concepts.qll +++ b/python/ql/src/experimental/semmle/python/Concepts.qll @@ -14,16 +14,35 @@ private import semmle.python.dataflow.new.RemoteFlowSources private import semmle.python.dataflow.new.TaintTracking private import experimental.semmle.python.Frameworks +/** Provides classes for modeling HTTP Header APIs. */ module HeaderDeclaration { + /** + * A data-flow node that collects functions setting HTTP Headers' content. + * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `HeaderDeclaration` instead. + */ abstract class Range extends DataFlow::Node { - abstract DataFlow::Node getHeaderInputNode(); + /** + * Gets the argument containing the header value. + */ + abstract DataFlow::Node getHeaderInput(); } } +/** + * A data-flow node that collects functions setting HTTP Headers' content. + * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `HeaderDeclaration` instead. + */ class HeaderDeclaration extends DataFlow::Node { HeaderDeclaration::Range range; HeaderDeclaration() { this = range } - DataFlow::Node getHeaderInputNode() { result = range.getHeaderInputNode() } + /** + * Gets the argument containing the header value. + */ + DataFlow::Node getHeaderInput() { result = range.getHeaderInput() } } diff --git a/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll index 1215710c1aa..e049caa2609 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll @@ -4,12 +4,15 @@ import semmle.python.dataflow.new.DataFlow import semmle.python.dataflow.new.TaintTracking import semmle.python.dataflow.new.RemoteFlowSources +/** + * A taint-tracking configuration for detecting HTTP Header injections. + */ class HeaderInjectionFlowConfig extends TaintTracking::Configuration { HeaderInjectionFlowConfig() { this = "HeaderInjectionFlowConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { - sink = any(HeaderDeclaration headerDeclaration).getHeaderInputNode() + sink = any(HeaderDeclaration headerDeclaration).getHeaderInput() } } From 017a778a20d8980700966d54f98b6041b78ad08f Mon Sep 17 00:00:00 2001 From: jorgectf Date: Fri, 18 Jun 2021 20:21:11 +0200 Subject: [PATCH 15/16] Polish make_response and fix extend argument --- .../src/experimental/semmle/python/frameworks/Flask.qll | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll index 4ed3ed58e3c..7ad1aa2e358 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll @@ -21,10 +21,9 @@ module ExperimentalFlask { * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.make_response */ private API::Node flaskMakeResponse() { - result in [ - API::moduleImport("flask").getMember("make_response"), - Flask::FlaskApp::instance().getMember("make_response") - ] + result = + [API::moduleImport("flask"), Flask::FlaskApp::instance()] + .getMember(["make_response", "jsonify", "make_default_options_response"]) } /** Gets a reference to a header instance. */ @@ -63,7 +62,7 @@ module ExperimentalFlask { private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range { FlaskMakeResponseExtend() { this.getFunction() = headerInstanceCall() } - override DataFlow::Node getHeaderInput() { result = this.getArg(0) } + override DataFlow::Node getHeaderInput() { result = this.getArg(_) } } private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range { From b10ade17beeeefe3b5811f8b7a97c60b8b14555a Mon Sep 17 00:00:00 2001 From: jorgectf Date: Sun, 20 Jun 2021 00:13:59 +0200 Subject: [PATCH 16/16] Update HeaderDeclaration input naming --- python/ql/src/experimental/semmle/python/Concepts.qll | 4 ++-- .../ql/src/experimental/semmle/python/frameworks/Django.qll | 4 ++-- .../ql/src/experimental/semmle/python/frameworks/Flask.qll | 6 +++--- .../src/experimental/semmle/python/frameworks/Werkzeug.qll | 2 +- .../semmle/python/security/injection/HTTPHeaders.qll | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll index 309e40b4471..a3d2104df52 100644 --- a/python/ql/src/experimental/semmle/python/Concepts.qll +++ b/python/ql/src/experimental/semmle/python/Concepts.qll @@ -159,7 +159,7 @@ module HeaderDeclaration { /** * Gets the argument containing the header value. */ - abstract DataFlow::Node getHeaderInput(); + abstract DataFlow::Node getAnInput(); } } @@ -177,5 +177,5 @@ class HeaderDeclaration extends DataFlow::Node { /** * Gets the argument containing the header value. */ - DataFlow::Node getHeaderInput() { result = range.getHeaderInput() } + DataFlow::Node getAnInput() { result = range.getAnInput() } } diff --git a/python/ql/src/experimental/semmle/python/frameworks/Django.qll b/python/ql/src/experimental/semmle/python/frameworks/Django.qll index 3353881fff8..68153dfae00 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Django.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Django.qll @@ -56,7 +56,7 @@ private module PrivateDjango { class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range { DjangoResponseSetItemCall() { this.getFunction() = headerSetItemCall() } - override DataFlow::Node getHeaderInput() { result = this.getArg([0, 1]) } + override DataFlow::Node getAnInput() { result = this.getArg([0, 1]) } } class DjangoResponseDefinition extends DataFlow::Node, HeaderDeclaration::Range { @@ -67,7 +67,7 @@ private module PrivateDjango { headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue() } - override DataFlow::Node getHeaderInput() { + override DataFlow::Node getAnInput() { result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()] } } diff --git a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll index 7ad1aa2e358..a62c38bb060 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll @@ -54,7 +54,7 @@ module ExperimentalFlask { headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue() } - override DataFlow::Node getHeaderInput() { + override DataFlow::Node getAnInput() { result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()] } } @@ -62,12 +62,12 @@ module ExperimentalFlask { private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range { FlaskMakeResponseExtend() { this.getFunction() = headerInstanceCall() } - override DataFlow::Node getHeaderInput() { result = this.getArg(_) } + override DataFlow::Node getAnInput() { result = this.getArg(_) } } private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range { FlaskResponse() { this = Flask::Response::classRef().getACall() } - override DataFlow::Node getHeaderInput() { result = this.getArgByName("headers") } + override DataFlow::Node getAnInput() { result = this.getArgByName("headers") } } } diff --git a/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll b/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll index 4c7e68f165a..d37fefe2af8 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Werkzeug.qll @@ -24,7 +24,7 @@ private module Werkzeug { this.getFunction().(DataFlow::AttrRead).getAttributeName() = "add" } - override DataFlow::Node getHeaderInput() { result = this.getArg(_) } + override DataFlow::Node getAnInput() { result = this.getArg(_) } } } } diff --git a/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll index e049caa2609..d31a7d5ac9d 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll @@ -13,6 +13,6 @@ class HeaderInjectionFlowConfig extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { - sink = any(HeaderDeclaration headerDeclaration).getHeaderInput() + sink = any(HeaderDeclaration headerDeclaration).getAnInput() } }