From 2fb87e87e4661f5c9f2dd1b4b6a815d9c319d49d Mon Sep 17 00:00:00 2001 From: haby0 Date: Wed, 27 Oct 2021 22:20:38 +0800 Subject: [PATCH 1/2] add any file read --- .../Security/CWE-073/AnyFileRead.py | 39 ++++++++++ .../Security/CWE-073/AnyFileRead.qhelp | 38 ++++++++++ .../Security/CWE-073/AnyFileRead.ql | 20 +++++ .../Security/CWE-073/AnyFileReadLib.qll | 74 +++++++++++++++++++ .../Security/CWE-073/AnyFileRead.expected | 16 ++++ .../Security/CWE-073/AnyFileRead.py | 39 ++++++++++ .../Security/CWE-073/AnyFileRead.qlref | 1 + 7 files changed, 227 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-073/AnyFileRead.py create mode 100644 python/ql/src/experimental/Security/CWE-073/AnyFileRead.qhelp create mode 100644 python/ql/src/experimental/Security/CWE-073/AnyFileRead.ql create mode 100644 python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.qlref diff --git a/python/ql/src/experimental/Security/CWE-073/AnyFileRead.py b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.py new file mode 100644 index 000000000000..90ca580c80c0 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python +# -*- coding: UTF-8 -*- +""" +@Desc ：Any File Injection +""" +from flask import Flask, send_file, make_response +from flask import request +import os + +filenames = ["/home/work/temp/a.png", "/home/work/temp/b.png"] + +app = Flask(__name__) + +@app.route('/bad1') +def bad1(): + filename = request.args.get('filename') + context = send_file(filename, as_attachment=True) # Bad + return context + +@app.route('/bad2') +def bad2(): + filename = request.args.get('filename') + context = open(filename, 'r') # Bad + response = make_response(context) + return response + +@app.route('/good1') +def good1(): + filename = request.args.get('filename') + if filename in filenames: # Good + context = open(filename, 'r') + else: + context = "filename error" + response = make_response(context) + return response + +if __name__ == '__main__': + app.debug = True + app.run() \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-073/AnyFileRead.qhelp b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.qhelp new file mode 100644 index 000000000000..5de15edba61b --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.qhelp @@ -0,0 +1,38 @@ + + + + +

+Accessing files using paths constructed from user-controlled data can allow an attacker to access +unexpected resources. This may cause sensitive information to be leaked +

+ + + +

+After the user completes the file path construction and before reading the file content, the file +path needs to be strictly verified. +

+ + + + +

+In the first and second examples, the accessed file name comes from the user's input, and without +any verification, it is directly returned to the user, causing the leakage of sensitive information. +For example: Linux system, the file name can enter "/etc/passwd" to obtain the system user password file. +

+ +

+In the third example, the file name entered by the user is fully verified, and there is no arbitrary file reading vulnerability. +

+ + + + + +
  • OWASP: Path Traversal.
    • +     +     diff --git a/python/ql/src/experimental/Security/CWE-073/AnyFileRead.ql b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.ql new file mode 100644 index 000000000000..1fda035a84a5 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-073/AnyFileRead.ql @@ -0,0 +1,20 @@ +/** + * @name Any File Read + * @description Accessing files using paths constructed by user-controlled data may allow attackers to access + * Unexpected resources, leading to leakage of sensitive information + * @kind path-problem + * @problem.severity error + * @precision high + * @id py/any-file-read + * @tags security + * external/cwe/cwe-073 + */ + +import python +import DataFlow::PathGraph +import AnyFileReadLib + +from AnyFileReadFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Any file read might include code from $@.", source.getNode(), + "this user input" diff --git a/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll b/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll new file mode 100644 index 000000000000..5d0275cbee5a --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll @@ -0,0 +1,74 @@ +import python +import semmle.python.Concepts +import semmle.python.ApiGraphs +import semmle.python.dataflow.new.TaintTracking +import semmle.python.dataflow.new.TaintTracking2 +import semmle.python.dataflow.new.RemoteFlowSources + +/** + * A taint-tracking configuration for tracking untrusted user input used in file read. + */ +class AnyFileReadFlowConfig extends TaintTracking::Configuration { + AnyFileReadFlowConfig() { this = "AnyFileReadFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof AnyFileReadSink } + + override predicate isSanitizer(DataFlow::Node node) { + exists(Compare compare | + ( + compare.getOp(0) instanceof In or + compare.getOp(0) instanceof NotIn + ) and + compare.getLeft() = node.asExpr() + ) + } +} + +abstract private class FileReadCall extends DataFlow::CallCfgNode { } + +private class FlaskSendFileCall extends FileReadCall { + FlaskSendFileCall() { this = API::moduleImport("flask").getMember("send_file").getACall() } +} + +private class OpenReadCall extends FileReadCall { + OpenReadCall() { + ( + this = API::builtin("open").getACall() or + this = API::moduleImport("io").getMember("open").getACall() + ) and + this.getArg(1).asExpr().(StrConst).getText().toLowerCase().indexOf("r") > -1 + } +} + +/** A data flow sink for any file read vulnerabilities. */ +class AnyFileReadSink extends DataFlow::Node { + AnyFileReadSink() { + exists(FileReadCall frc, DataFlow::Node pred | frc = pred | + frc.getArg(0) = this and + any(ResponseFlowConfig rfc).hasFlow(pred, _) + ) + } +} + +/** + * A taint-tracking configuration for file content to http response. + */ +private class ResponseFlowConfig extends TaintTracking2::Configuration { + ResponseFlowConfig() { this = "ResponseFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof FileReadCall } + + override predicate isSink(DataFlow::Node sink) { + sink = any(HTTP::Server::HttpResponse hr).getBody() + } + + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(DataFlow::CallCfgNode call | + call = API::moduleImport("falsk").getMember("make_response").getACall() and + call.getArg(0) = node1 and + call = node2 + ) + } +} diff --git a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected new file mode 100644 index 000000000000..62e467be80c1 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected @@ -0,0 +1,16 @@ +edges +| AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | +| AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | +| AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | +| AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | +nodes +| AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +| AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +subpaths +#select +| AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | this user input | +| AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | this user input | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py new file mode 100644 index 000000000000..90ca580c80c0 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python +# -*- coding: UTF-8 -*- +""" +@Desc ：Any File Injection +""" +from flask import Flask, send_file, make_response +from flask import request +import os + +filenames = ["/home/work/temp/a.png", "/home/work/temp/b.png"] + +app = Flask(__name__) + +@app.route('/bad1') +def bad1(): + filename = request.args.get('filename') + context = send_file(filename, as_attachment=True) # Bad + return context + +@app.route('/bad2') +def bad2(): + filename = request.args.get('filename') + context = open(filename, 'r') # Bad + response = make_response(context) + return response + +@app.route('/good1') +def good1(): + filename = request.args.get('filename') + if filename in filenames: # Good + context = open(filename, 'r') + else: + context = "filename error" + response = make_response(context) + return response + +if __name__ == '__main__': + app.debug = True + app.run() \ No newline at end of file diff --git a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.qlref b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.qlref new file mode 100644 index 000000000000..7b3e5a704d3f --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-073/AnyFileRead.ql \ No newline at end of file From 8e0f5b3284d3f5dcc3bb812c7964f26b6b076317 Mon Sep 17 00:00:00 2001 From: haby0 Date: Thu, 28 Oct 2021 15:40:44 +0800 Subject: [PATCH 2/2] Modify Blueprint module --- .../ql/lib/semmle/python/frameworks/Flask.qll | 5 ++++- .../Security/CWE-073/AnyFileReadLib.qll | 2 +- .../Security/CWE-073/AnyFileRead.expected | 22 +++++++++++-------- .../Security/CWE-073/AnyFileRead.py | 15 +++++++++++++ 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Flask.qll b/python/ql/lib/semmle/python/frameworks/Flask.qll index 3c2cc5af5ecf..46f20a63c18c 100644 --- a/python/ql/lib/semmle/python/frameworks/Flask.qll +++ b/python/ql/lib/semmle/python/frameworks/Flask.qll @@ -73,7 +73,10 @@ module Flask { */ module Blueprint { /** Gets a reference to the `flask.Blueprint` class. */ - API::Node classRef() { result = API::moduleImport("flask").getMember("Blueprint") } + API::Node classRef() { + result = API::moduleImport("flask").getMember("Blueprint") or + result = API::moduleImport("flask").getMember("blueprints").getMember("Blueprint") + } /** Gets a reference to an instance of `flask.Blueprint`. */ API::Node instance() { result = classRef().getReturn() } diff --git a/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll b/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll index 5d0275cbee5a..eceea625d7ac 100644 --- a/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll +++ b/python/ql/src/experimental/Security/CWE-073/AnyFileReadLib.qll @@ -66,7 +66,7 @@ private class ResponseFlowConfig extends TaintTracking2::Configuration { override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { exists(DataFlow::CallCfgNode call | - call = API::moduleImport("falsk").getMember("make_response").getACall() and + call = API::moduleImport("flask").getMember("make_response").getACall() and call.getArg(0) = node1 and call = node2 ) diff --git a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected index 62e467be80c1..693b10776141 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.expected @@ -1,16 +1,20 @@ edges -| AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | -| AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | -| AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | +| AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | AnyFileRead.py:23:25:23:32 | ControlFlowNode for filename | +| AnyFileRead.py:28:16:28:22 | ControlFlowNode for request | AnyFileRead.py:28:16:28:27 | ControlFlowNode for Attribute | +| AnyFileRead.py:28:16:28:27 | ControlFlowNode for Attribute | AnyFileRead.py:29:20:29:27 | ControlFlowNode for filename | +| AnyFileRead.py:34:10:34:17 | ControlFlowNode for filename | AnyFileRead.py:38:30:38:38 | ControlFlowNode for file_path | nodes -| AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | -| AnyFileRead.py:16:16:16:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | -| AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | | AnyFileRead.py:22:16:22:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | -| AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +| AnyFileRead.py:23:25:23:32 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +| AnyFileRead.py:28:16:28:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request | +| AnyFileRead.py:28:16:28:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute | +| AnyFileRead.py:29:20:29:27 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +| AnyFileRead.py:34:10:34:17 | ControlFlowNode for filename | semmle.label | ControlFlowNode for filename | +| AnyFileRead.py:38:30:38:38 | ControlFlowNode for file_path | semmle.label | ControlFlowNode for file_path | subpaths #select -| AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | AnyFileRead.py:17:25:17:32 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:16:16:16:22 | ControlFlowNode for request | this user input | -| AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | AnyFileRead.py:23:20:23:27 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | this user input | +| AnyFileRead.py:23:25:23:32 | ControlFlowNode for filename | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | AnyFileRead.py:23:25:23:32 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:22:16:22:22 | ControlFlowNode for request | this user input | +| AnyFileRead.py:29:20:29:27 | ControlFlowNode for filename | AnyFileRead.py:28:16:28:22 | ControlFlowNode for request | AnyFileRead.py:29:20:29:27 | ControlFlowNode for filename | Any file read might include code from $@. | AnyFileRead.py:28:16:28:22 | ControlFlowNode for request | this user input | +| AnyFileRead.py:38:30:38:38 | ControlFlowNode for file_path | AnyFileRead.py:34:10:34:17 | ControlFlowNode for filename | AnyFileRead.py:38:30:38:38 | ControlFlowNode for file_path | Any file read might include code from $@. | AnyFileRead.py:34:10:34:17 | ControlFlowNode for filename | this user input | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py index 90ca580c80c0..2cb697f7f508 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py +++ b/python/ql/test/experimental/query-tests/Security/CWE-073/AnyFileRead.py @@ -5,8 +5,14 @@ """ from flask import Flask, send_file, make_response from flask import request +from flask.blueprints import Blueprint import os +blueprint = Blueprint('routes_general', + __name__, + static_folder='../static', + template_folder='../templates') + filenames = ["/home/work/temp/a.png", "/home/work/temp/b.png"] app = Flask(__name__) @@ -24,6 +30,15 @@ def bad2(): response = make_response(context) return response +@blueprint.route('/note_attachment/') +def bad3(filename): + file_path = os.path.join(PATH_NOTE_ATTACHMENTS, filename) + if file_path is not None: + try: + return send_file(file_path, as_attachment=True) + except Exception: + logger.exception("Send note attachment") + @app.route('/good1') def good1(): filename = request.args.get('filename')