From cc0e2fc56035ffdbb3592c413192577ebc34c262 Mon Sep 17 00:00:00 2001 From: Thank You Date: Wed, 7 Apr 2021 22:22:12 -0400 Subject: [PATCH 1/6] Add initial code for SQLAlchemy injection query --- .../CWE-089/SqlAlchemyInjection.qhelp | 36 ++++++++++ .../Security/CWE-089/SqlAlchemyInjection.ql | 19 +++++ .../experimental/semmle/python/Concepts.qll | 14 ++++ .../semmle/python/frameworks/Stdlib.qll | 14 ++++ .../injection/SQLAlchemyInjection.qll | 72 +++++++++++++++++++ 5 files changed, 155 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.qhelp create mode 100644 python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql create mode 100644 python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll diff --git a/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.qhelp b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.qhelp new file mode 100644 index 000000000000..8a23c5c6a59d --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.qhelp @@ -0,0 +1,36 @@ + + + + +

+ Passing user-controlled sources into SQL queries used in SQLAlchemy methods can result in a SQL injection flaw. + This tainted SQL query containing a user-controlled source when passed into a SQLAlchemy method can execute a malicious query in a SQL database such as Postgres, MySQL, or SQLite. +

+ +

+ Because a user-controlled source is passed into the query, the malicious user can have complete control over the query itself. + When the tainted query is executed, the malicious user can commit malicious actions such as bypassing role restrictions or accessing and modifying restricted data in the SQL database. +

+
+ + +

+ SQL injections can be prevented by escaping user-input's special characters that are passed into the SQL query from the user-supplied source. + Alternatively, using a sanitize library such as Sqlescapy will ensure that user-supplied sources can not act as a malicious query. +

+ + + +

In the example below, the user-supplied source is passed to a SQLAlchemy method that queries the SQLite database.

+ +

This can be fixed by using a sanitizer library like Sqlescapy as shown in this annotated code version below.

+ + + + +
  • OWASP: SQL Injection
  • +
  • SQLAlchemy: Documentation
  • +
    +
    diff --git a/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql new file mode 100644 index 000000000000..be3543eced4d --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql @@ -0,0 +1,19 @@ +/** + * @name SqlAlchemy Injection + * @description Building a SQL query from user-controlled sources is vulnerable to insertion of + * malicious SQL code by the user in SQLAlchemy. + * @kind path-problem + * @problem.severity error + * @id python/sqlalchemy-injection + * @tags experimental + * security + * external/cwe/cwe-089 + */ + +import python +import experimental.semmle.python.security.injection.SqlAlchemyInjection + +from CustomPathNode source, CustomPathNode sink +where SqlAlchemyInjectionFlow(source, sink) +select sink, source, sink, "$@ SQL query contains an unsanitized $@", sink, "This", source, + "user-provided value" diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll index 904b7967ee87..c15415e166c3 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 SqlSanitizer { + abstract class Range extends DataFlow::Node { + abstract DataFlow::Node getSanitizerNode(); + } +} + +class SqlSanitizer extends DataFlow::Node { + SqlSanitizer::Range range; + + SqlSanitizer() { this = range } + + DataFlow::Node getSanitizerNode() { result = range.getSanitizerNode() } +} diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll index 420caf0d73bb..419923069008 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll @@ -8,4 +8,18 @@ private import semmle.python.dataflow.new.DataFlow private import semmle.python.dataflow.new.TaintTracking private import semmle.python.dataflow.new.RemoteFlowSources private import experimental.semmle.python.Concepts +private import semmle.python.Concepts private import semmle.python.ApiGraphs + +private module SQL { + // TODO: We need to add the SqlExecution::Range subclasses that cover SqlAlchemy sinks + + private class SqlSanitizerCall extends DataFlow::CallCfgNode, NoSQLSanitizer::Range { + SqlSanitizerCall() { + this = + API::moduleImport("sqlescapy").getMember("sqlescape").getACall() + } + + override DataFlow::Node getSanitizerNode() { result = this.getArg(0) } + } +} diff --git a/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll b/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll new file mode 100644 index 000000000000..d5bd59db6f42 --- /dev/null +++ b/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll @@ -0,0 +1,72 @@ +import python +import semmle.python.dataflow.new.DataFlow +import semmle.python.dataflow.new.DataFlow2 +import semmle.python.dataflow.new.TaintTracking +import semmle.python.dataflow.new.TaintTracking2 +import experimental.semmle.python.Concepts +import semmle.python.dataflow.new.RemoteFlowSources +import semmle.python.ApiGraphs +import semmle.python.security.dataflow.ChainedConfigs12 + +class JsonLoadsCall extends DataFlow::CallCfgNode { + JsonLoadsCall() { this = API::moduleImport("json").getMember("loads").getACall() } + + DataFlow::Node getLoadNode() { result = this.getArg(0) } +} + +class XmlToDictParseCall extends DataFlow::CallCfgNode { + XmlToDictParseCall() { this = API::moduleImport("xmltodict").getMember("parse").getACall() } + + DataFlow::Node getParseNode() { result = this.getArg(0) } +} + +class UltraJsonLoadsCall extends DataFlow::CallCfgNode { + UltraJsonLoadsCall() { this = API::moduleImport("ujson").getMember("loads").getACall() } + + DataFlow::Node getLoadNode() { result = this.getArg(0) } +} + +class DataToDictSink extends DataFlow::Node { + DataToDictSink() { + this = any(JsonLoadsCall jsonLoads).getLoadNode() or + this = any(XmlToDictParseCall jsonLoads).getParseNode() or + this = any(UltraJsonLoadsCall jsonLoads).getLoadNode() + } +} + +class RFSToDictConfig extends TaintTracking::Configuration { + RFSToDictConfig() { this = "RFSToDictConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof DataToDictSink } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + sanitizer = any(SqlSanitizer SqlSanitizer).getSanitizerNode() + } +} + +class FromDataDictToSink extends TaintTracking2::Configuration { + FromDataDictToSink() { this = "FromDataDictToSink" } + + override predicate isSource(DataFlow::Node source) { source instanceof DataToDictSink } + + override predicate isSink(DataFlow::Node sink) { + sink = any(SqlAlchemyQuery SqlAlchemyQuery).getQueryNode() + } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + sanitizer = any(SqlSanitizer SqlSanitizer).getSanitizerNode() + } +} + +predicate SqlAlchemyInjectionFlow(CustomPathNode source, CustomPathNode sink) { + exists( + RFSToDictConfig config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2, + FromDataDictToSink config2 + | + config.hasFlowPath(source.asNode1(), mid1) and + config2.hasFlowPath(mid2, sink.asNode2()) and + mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode() + ) +} From 445c14af16d5bd2e002e3a3b5bb82f326fe8902b Mon Sep 17 00:00:00 2001 From: thank_you Date: Thu, 8 Apr 2021 18:00:00 -0400 Subject: [PATCH 2/6] Add query tests for CWE-089 --- .../Security/CWE-089/sqlalchemy_bad.py | 33 ++++++++++++++++ .../Security/CWE-089/sqlalchemy_flask_bad.py | 36 ++++++++++++++++++ .../Security/CWE-089/sqlalchemy_flask_good.py | 38 +++++++++++++++++++ .../Security/CWE-089/sqlalchemy_good.py | 37 ++++++++++++++++++ 4 files changed, 144 insertions(+) create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_bad.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_good.py diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_bad.py new file mode 100644 index 000000000000..d5a6cf27e0ea --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_bad.py @@ -0,0 +1,33 @@ +import sqlalchemy +from sqlalchemy import Column, Integer, String, ForeignKey, create_engine +from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy.orm import relationship, backref, sessionmaker, joinedload +from sqlalchemy.sql import text +from sqlescapy import sqlescape + + +engine = create_engine('sqlite:///:memory:', echo=True) +Base = declarative_base() + +class User(Base): + __tablename__ = 'users' + + id = Column(Integer, primary_key=True) + name = Column(String) + +Base.metadata.create_all(engine) + +Session = sessionmaker(bind=engine) +session = Session() + +ed_user = User(name='ed') +ed_user2 = User(name='george') + +session.add(ed_user) +session.add(ed_user2) + +session.commit() + +malicious_user_input = sys.argv[1] +malicious_user_input = text("name='{}'".format(malicious_user_input)) +session.query(User).filter(malicious_user_input).count() diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py new file mode 100644 index 000000000000..d2988c421420 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py @@ -0,0 +1,36 @@ +from flask import Flask, request +from flask_sqlalchemy import SQLAlchemy +import json +from sqlalchemy.sql import text + + +app = Flask(__name__) +app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///:memory:' +app.config['SQLALCHEMY_ECHO'] = True +db = SQLAlchemy(app) + +class User(db.Model): + uid = db.Column(db.Integer, primary_key=True, autoincrement=True) + email = db.Column(db.String(50)) + + def __init__(self, email): + self.email = email + +db.create_all() + +user1 = User(email='admin@example.com') +user2 = User(email='admin2@example.com') +db.session.add(user1) +db.session.add(user2) +db.session.commit() + +@app.route('/') +def user_count(): + user_info = json.loads(request.args['user_info']) + sql_query = "email='{}'".format(user_info['email']) + user_count = db.session.query(User).filter(sql_query).count() + + return "Users found: {}".format(user_count) + +if __name__ == '__main__': + app.run(debug=True) diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py new file mode 100644 index 000000000000..5375217bb472 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py @@ -0,0 +1,38 @@ +from flask import Flask, request +from flask_sqlalchemy import SQLAlchemy +import json +from sqlalchemy.sql import text +from sqlescapy import sqlescape + + +app = Flask(__name__) +app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///:memory:' +app.config['SQLALCHEMY_ECHO'] = True +db = SQLAlchemy(app) + +class User(db.Model): + uid = db.Column(db.Integer, primary_key=True, autoincrement=True) + email = db.Column(db.String(50)) + + def __init__(self, email): + self.email = email + +db.create_all() + +user1 = User(email='admin@example.com') +user2 = User(email='admin2@example.com') +db.session.add(user1) +db.session.add(user2) +db.session.commit() + +@app.route('/') +def user_count(): + user_info = json.loads(request.args['user_info']) + sanitized_input = sqlescape(user_info['email'])) + sql_query = "email='{}'".format(sanitized_input) + user_count = db.session.query(User).filter(sql_query).count() + + return "Users found: {}".format(user_count) + +if __name__ == '__main__': + app.run(debug=True) diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_good.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_good.py new file mode 100644 index 000000000000..08de4264181c --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_good.py @@ -0,0 +1,37 @@ +import sqlalchemy +from sqlalchemy import Column, Integer, String, ForeignKey, create_engine +from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy.orm import relationship, backref, sessionmaker, joinedload +from sqlalchemy.sql import text +from sqlescapy import sqlescape + + +engine = create_engine('sqlite:///:memory:', echo=True) +Base = declarative_base() + +class User(Base): + __tablename__ = 'users' + + id = Column(Integer, primary_key=True) + name = Column(String) + +Base.metadata.create_all(engine) + +Session = sessionmaker(bind=engine) +session = Session() + +ed_user = User(name='ed') +ed_user2 = User(name='george') + +session.add(ed_user) +session.add(ed_user2) + +session.commit() + + +malicious_user_input = sys.argv[1] +escaped_input = sqlescape(malicious_user_input) +sql_text = "name='{}'".format(escaped_input) +sql_query = text(sql_text) + +session.query(User).filter(malicious_user_input).count() From 95931a45a56a67d0edf9d23e3a7f6f5c1e63c51d Mon Sep 17 00:00:00 2001 From: thank_you Date: Thu, 8 Apr 2021 18:02:33 -0400 Subject: [PATCH 3/6] Comment out app.run for flask examples --- .../query-tests/Security/CWE-089/sqlalchemy_flask_bad.py | 4 ++-- .../query-tests/Security/CWE-089/sqlalchemy_flask_good.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py index d2988c421420..eafdb67d877d 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_bad.py @@ -32,5 +32,5 @@ def user_count(): return "Users found: {}".format(user_count) -if __name__ == '__main__': - app.run(debug=True) +# if __name__ == '__main__': +# app.run(debug=True) diff --git a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py index 5375217bb472..c871d1c8d259 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py +++ b/python/ql/test/experimental/query-tests/Security/CWE-089/sqlalchemy_flask_good.py @@ -34,5 +34,5 @@ def user_count(): return "Users found: {}".format(user_count) -if __name__ == '__main__': - app.run(debug=True) +# if __name__ == '__main__': +# app.run(debug=True) From e8988b97039942663d250467f28e431342262f94 Mon Sep 17 00:00:00 2001 From: thank_you Date: Mon, 12 Apr 2021 20:09:43 -0400 Subject: [PATCH 4/6] Add predicates for finding SQLAlchemy sinks --- .../semmle/python/frameworks/Stdlib.qll | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll index 419923069008..f70a45f5ef0c 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll @@ -11,8 +11,43 @@ private import experimental.semmle.python.Concepts private import semmle.python.Concepts private import semmle.python.ApiGraphs -private module SQL { - // TODO: We need to add the SqlExecution::Range subclasses that cover SqlAlchemy sinks +private module SQLAlchemy { + private class SqlAlchemyQuerySinkMethods extends string { + SqlAlchemyQuerySinkMethods() { this in [ + "filter", "filter_by", "having", "group_by", "order_by" + ] + } + } + + private API::Node sqlAlchemySessionInstance() { + result = API::moduleImport("sqlalchemy.orm").getMember("Session").getReturn() or + result = API::moduleImport("sqlalchemy.orm").getMember("sessionmaker").getReturn().getReturn() + } + + private API::Node sqlAlchemyQueryInstance() { + result = sqlAlchemySessionInstance().getMember("query").getReturn() + } + + private API::Node sqlAlchemyEngineInstance() { + result = API::moduleImport("sqlalchemy").getMember("create_engine").getReturn() + } + + private class SqlAlchemyQuerySink extends DataFlow::CallCfgNode, SqlExecution::Range { + DataFlow::Node sql; + + SqlAlchemyQuerySink() { + this in [ + sqlAlchemyQueryInstance().getMember(any(SqlAlchemyQuerySinkMethods sinkMethod)).getACall(), + sqlAlchemySessionInstance().getMember("execute").getACall(), + sqlAlchemySessionInstance().getMember("scalar").getACall() + sqlAlchemyEngineInstance().getMember("connect").getReturn().getMember("execute").getACall(), + sqlAlchemyEngineInstance().getMember("begin").getReturn().getMember("execute").getACall() + ] and + sql = this.getArg(0) + } + + override DataFlow::Node getSql() { result = sql } + } private class SqlSanitizerCall extends DataFlow::CallCfgNode, NoSQLSanitizer::Range { SqlSanitizerCall() { From 5c39466911fad99a25255379fe7f7484ce374013 Mon Sep 17 00:00:00 2001 From: thank_you Date: Mon, 12 Apr 2021 20:22:50 -0400 Subject: [PATCH 5/6] Break up query sink logic into separate predicates --- .../semmle/python/frameworks/Stdlib.qll | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll index f70a45f5ef0c..c3ce91ae7430 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll @@ -32,21 +32,39 @@ private module SQLAlchemy { result = API::moduleImport("sqlalchemy").getMember("create_engine").getReturn() } - private class SqlAlchemyQuerySink extends DataFlow::CallCfgNode, SqlExecution::Range { + private class SqlAlchemyInjectionSink extends DataFlow::CallCfgNode, SqlExecution::Range { DataFlow::Node sql; - SqlAlchemyQuerySink() { + SqlAlchemyInjectionSink() { + this in [ + any(SqlAlchemyExecuteInjectionSink injectionSink) + any(SqlAlchemyQueryInjectionSink injectionSink) + ] and + sql = this.getArg(0) + } + + override DataFlow::Node getSql() { result = sql } + } + + private class SqlAlchemyExecuteInjectionSink extends DataFlow::Node { + SqlAlchemyExecuteInjectionSink() { this in [ - sqlAlchemyQueryInstance().getMember(any(SqlAlchemyQuerySinkMethods sinkMethod)).getACall(), sqlAlchemySessionInstance().getMember("execute").getACall(), - sqlAlchemySessionInstance().getMember("scalar").getACall() sqlAlchemyEngineInstance().getMember("connect").getReturn().getMember("execute").getACall(), sqlAlchemyEngineInstance().getMember("begin").getReturn().getMember("execute").getACall() ] and sql = this.getArg(0) } + } - override DataFlow::Node getSql() { result = sql } + private class SqlAlchemyQueryInjectionSink extends DataFlow::Node { + SqlAlchemyQueryInjectionSink() { + this in [ + sqlAlchemyQueryInstance().getMember(any(SqlAlchemyQuerySinkMethods sinkMethod)).getACall(), + sqlAlchemySessionInstance().getMember("scalar").getACall() + ] and + sql = this.getArg(0) + } } private class SqlSanitizerCall extends DataFlow::CallCfgNode, NoSQLSanitizer::Range { From 492d7da0f93456e3877591ea697115f43d662dc5 Mon Sep 17 00:00:00 2001 From: thank_you Date: Mon, 12 Apr 2021 20:24:43 -0400 Subject: [PATCH 6/6] Create SqlAlchemyInjectionConfig class --- .../Security/CWE-089/SqlAlchemyInjection.ql | 4 +- .../injection/SQLAlchemyInjection.qll | 61 ++----------------- 2 files changed, 6 insertions(+), 59 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql index be3543eced4d..bfca66259fc4 100644 --- a/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql +++ b/python/ql/src/experimental/Security/CWE-089/SqlAlchemyInjection.ql @@ -13,7 +13,7 @@ import python import experimental.semmle.python.security.injection.SqlAlchemyInjection -from CustomPathNode source, CustomPathNode sink -where SqlAlchemyInjectionFlow(source, sink) +from DataFlow::PathNode source, DataFlow::PathNode sink, SqlAlchemyInjectionConfig sqlAlchemyInjectionConfig +where sqlAlchemyInjectionConfig.hasFlowPath(source, sink) select sink, source, sink, "$@ SQL query contains an unsanitized $@", sink, "This", source, "user-provided value" diff --git a/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll b/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll index d5bd59db6f42..32b98c5957d8 100644 --- a/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll +++ b/python/ql/src/experimental/semmle/python/security/injection/SQLAlchemyInjection.qll @@ -1,72 +1,19 @@ import python import semmle.python.dataflow.new.DataFlow -import semmle.python.dataflow.new.DataFlow2 import semmle.python.dataflow.new.TaintTracking -import semmle.python.dataflow.new.TaintTracking2 import experimental.semmle.python.Concepts import semmle.python.dataflow.new.RemoteFlowSources import semmle.python.ApiGraphs -import semmle.python.security.dataflow.ChainedConfigs12 +import experimental.semmle.python.frameworks.Stdlib -class JsonLoadsCall extends DataFlow::CallCfgNode { - JsonLoadsCall() { this = API::moduleImport("json").getMember("loads").getACall() } - - DataFlow::Node getLoadNode() { result = this.getArg(0) } -} - -class XmlToDictParseCall extends DataFlow::CallCfgNode { - XmlToDictParseCall() { this = API::moduleImport("xmltodict").getMember("parse").getACall() } - - DataFlow::Node getParseNode() { result = this.getArg(0) } -} - -class UltraJsonLoadsCall extends DataFlow::CallCfgNode { - UltraJsonLoadsCall() { this = API::moduleImport("ujson").getMember("loads").getACall() } - - DataFlow::Node getLoadNode() { result = this.getArg(0) } -} - -class DataToDictSink extends DataFlow::Node { - DataToDictSink() { - this = any(JsonLoadsCall jsonLoads).getLoadNode() or - this = any(XmlToDictParseCall jsonLoads).getParseNode() or - this = any(UltraJsonLoadsCall jsonLoads).getLoadNode() - } -} - -class RFSToDictConfig extends TaintTracking::Configuration { - RFSToDictConfig() { this = "RFSToDictConfig" } +class SqlAlchemyInjectionConfig extends TaintTracking::Configuration { + SqlAlchemyInjectionConfig() { this = "SqlAlchemyInjectionConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - override predicate isSink(DataFlow::Node sink) { sink instanceof DataToDictSink } + override predicate isSink(DataFlow::Node sink) { sink instanceof SqlAlchemyInjectionSink } override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer = any(SqlSanitizer SqlSanitizer).getSanitizerNode() } } - -class FromDataDictToSink extends TaintTracking2::Configuration { - FromDataDictToSink() { this = "FromDataDictToSink" } - - override predicate isSource(DataFlow::Node source) { source instanceof DataToDictSink } - - override predicate isSink(DataFlow::Node sink) { - sink = any(SqlAlchemyQuery SqlAlchemyQuery).getQueryNode() - } - - override predicate isSanitizer(DataFlow::Node sanitizer) { - sanitizer = any(SqlSanitizer SqlSanitizer).getSanitizerNode() - } -} - -predicate SqlAlchemyInjectionFlow(CustomPathNode source, CustomPathNode sink) { - exists( - RFSToDictConfig config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2, - FromDataDictToSink config2 - | - config.hasFlowPath(source.asNode1(), mid1) and - config2.hasFlowPath(mid2, sink.asNode2()) and - mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode() - ) -}