Merge pull request #1203 from markshannon/python-taint-tracking-configuration-2

taus-semmle · web-flow · commit adf8cdcde583 · 2019-04-09T10:01:35.000+02:00
Python: Use taint tracking configuration for queries.
diff --git a/python/ql/src/Security/CWE-079/ReflectedXss.ql b/python/ql/src/Security/CWE-079/ReflectedXss.ql
@@ -25,6 +25,17 @@ import semmle.python.web.HttpResponse
 /* Flow */
 import semmle.python.security.strings.Untrusted
 
-from TaintedPathSource src, TaintedPathSink sink
-where src.flowsTo(sink)
+
+class ReflectedXssConfiguration extends TaintTracking::Configuration {
+
+    ReflectedXssConfiguration() { this = "Reflected XSS configuration" }
+
+    override predicate isSource(TaintTracking::Source source) { source instanceof HttpRequestTaintSource }
+
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpResponseTaintSink }
+
+}
+
+from ReflectedXssConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where config.hasFlowPath(src, sink)
 select sink.getSink(), src, sink, "Cross-site scripting vulnerability due to $@.", src.getSource(), "user-provided value"
diff --git a/python/ql/src/Security/CWE-089/SqlInjection.ql b/python/ql/src/Security/CWE-089/SqlInjection.ql
@@ -22,7 +22,16 @@ import semmle.python.security.injection.Sql
 import semmle.python.web.django.Db
 import semmle.python.web.django.Model
 
+class SQLInjectionConfiguration extends TaintTracking::Configuration {
 
-from TaintedPathSource src, TaintedPathSink sink
-where src.flowsTo(sink)
+    SQLInjectionConfiguration() { this = "SQL injection configuration" }
+
+    override predicate isSource(TaintTracking::Source source) { source instanceof HttpRequestTaintSource }
+
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof SqlInjectionSink }
+
+}
+
+from SQLInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where config.hasFlowPath(src, sink)
 select sink.getSink(), src, sink, "This SQL query depends on $@.", src.getSource(), "a user-provided value"
diff --git a/python/ql/src/Security/CWE-094/CodeInjection.ql b/python/ql/src/Security/CWE-094/CodeInjection.ql
@@ -23,7 +23,17 @@ import semmle.python.web.HttpRequest
 /* Sinks */
 import semmle.python.security.injection.Exec
 
+class CodeInjectionConfiguration extends TaintTracking::Configuration {
 
-from TaintedPathSource src, TaintedPathSink sink
-where src.flowsTo(sink)
+    CodeInjectionConfiguration() { this = "Code injection configuration" }
+
+    override predicate isSource(TaintTracking::Source source) { source instanceof HttpRequestTaintSource }
+
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof StringEvaluationNode }
+
+}
+
+
+from CodeInjectionConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where config.hasFlowPath(src, sink)
 select sink.getSink(), src, sink, "$@ flows to here and is interpreted as code.", src.getSource(), "User-provided value"
diff --git a/python/ql/src/Security/CWE-502/UnsafeDeserialization.ql b/python/ql/src/Security/CWE-502/UnsafeDeserialization.ql
@@ -24,7 +24,16 @@ import semmle.python.security.injection.Pickle
 import semmle.python.security.injection.Marshal
 import semmle.python.security.injection.Yaml
 
+class UnsafeDeserializationConfiguration  extends TaintTracking::Configuration {
 
-from TaintedPathSource src, TaintedPathSink sink
-where src.flowsTo(sink)
+    UnsafeDeserializationConfiguration() { this = "Unsafe deserialization configuration" }
+
+    override predicate isSource(TaintTracking::Source source) { source instanceof HttpRequestTaintSource }
+
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof DeserializationSink }
+
+}
+
+from UnsafeDeserializationConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where config.hasFlowPath(src, sink)
 select sink.getSink(), src, sink, "Deserializing of $@.", src.getSource(), "untrusted input"
diff --git a/python/ql/src/semmle/python/security/SQL.qll b/python/ql/src/semmle/python/security/SQL.qll
@@ -0,0 +1,4 @@
+import python
+import semmle.python.security.TaintTracking
+
+abstract class SqlInjectionSink extends TaintSink {}
diff --git a/python/ql/src/semmle/python/security/TaintTracking.qll b/python/ql/src/semmle/python/security/TaintTracking.qll
@@ -1668,3 +1668,4 @@ private predicate sequence_call(ControlFlowNode fromnode, CallNode tonode) {
         cls.refersTo(theSetType())
     )
 }
+
diff --git a/python/ql/src/semmle/python/security/injection/Deserialization.qll b/python/ql/src/semmle/python/security/injection/Deserialization.qll
@@ -0,0 +1,14 @@
+
+import python
+import semmle.python.security.TaintTracking
+
+
+/** `pickle.loads(untrusted)` vulnerability. */
+abstract class DeserializationSink extends TaintSink {
+
+    bindingset[this]
+    DeserializationSink() {
+        this = this
+    }
+
+}
diff --git a/python/ql/src/semmle/python/security/injection/Marshal.qll b/python/ql/src/semmle/python/security/injection/Marshal.qll
@@ -9,6 +9,7 @@ import python
 
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Untrusted
+import semmle.python.security.injection.Deserialization
 
 
 private FunctionObject marshalLoads() {
@@ -18,7 +19,7 @@ private FunctionObject marshalLoads() {
 
 /** A taint sink that is potentially vulnerable to malicious marshaled objects.
  * The `vuln` in `marshal.loads(vuln)`. */
-class UnmarshalingNode extends TaintSink {
+class UnmarshalingNode extends DeserializationSink {
 
     override string toString() { result = "unmarshaling vulnerability" }
 
diff --git a/python/ql/src/semmle/python/security/injection/Pickle.qll b/python/ql/src/semmle/python/security/injection/Pickle.qll
@@ -9,6 +9,7 @@ import python
 
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Untrusted
+import semmle.python.security.injection.Deserialization
 
 
 private ModuleObject pickleModule() {
@@ -24,7 +25,7 @@ private FunctionObject pickleLoads() {
 }
 
 /** `pickle.loads(untrusted)` vulnerability. */
-class UnpicklingNode extends TaintSink {
+class UnpicklingNode extends DeserializationSink {
 
     override string toString() { result = "unpickling untrusted data" }
 
diff --git a/python/ql/src/semmle/python/security/injection/Sql.qll b/python/ql/src/semmle/python/security/injection/Sql.qll
@@ -9,6 +9,7 @@ import python
 
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Untrusted
+import semmle.python.security.SQL
 
 
 private StringObject first_part(ControlFlowNode command) {
@@ -48,11 +49,10 @@ abstract class DbCursor extends TaintKind {
 
 }
 
-
 /** A part of a string that appears to be a SQL command and is thus
  * vulnerable to malicious input.
  */
-class SimpleSqlStringInjection extends TaintSink {
+class SimpleSqlStringInjection extends SqlInjectionSink {
 
     override string toString() { result = "simple SQL string injection" }
 
@@ -76,7 +76,7 @@ abstract class DbConnectionSource extends TaintSource {
 /** A taint sink that is vulnerable to malicious SQL queries.
  * The `vuln` in `db.connection.execute(vuln)` and similar.
  */
-class DbConnectionExecuteArgument extends TaintSink {
+class DbConnectionExecuteArgument extends SqlInjectionSink {
 
     override string toString() { result = "db.connection.execute" }
 
diff --git a/python/ql/src/semmle/python/security/injection/Xml.qll b/python/ql/src/semmle/python/security/injection/Xml.qll
@@ -8,6 +8,7 @@ import python
 
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Untrusted
+import semmle.python.security.injection.Deserialization
 
 
 private ModuleObject xmlElementTreeModule() {
@@ -73,7 +74,7 @@ class ExternalXmlString extends ExternalStringKind {
 /** A call to an XML library function that is potentially vulnerable to a
  * specially crafted XML string.
  */
-class XmlLoadNode extends TaintSink {
+class XmlLoadNode extends DeserializationSink {
 
     override string toString() { result = "xml.load vulnerability" }
 
diff --git a/python/ql/src/semmle/python/security/injection/Yaml.qll b/python/ql/src/semmle/python/security/injection/Yaml.qll
@@ -10,14 +10,15 @@ import python
 
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Untrusted
+import semmle.python.security.injection.Deserialization
 
 
 private FunctionObject yamlLoad() {
     result = ModuleObject::named("yaml").attr("load")
 }
 
 /** `yaml.load(untrusted)` vulnerability. */
-class YamlLoadNode extends TaintSink {
+class YamlLoadNode extends DeserializationSink {
 
     override string toString() { result = "yaml.load vulnerability" }
 
diff --git a/python/ql/src/semmle/python/web/Http.qll b/python/ql/src/semmle/python/web/Http.qll
@@ -3,11 +3,7 @@ import semmle.python.security.TaintTracking
 import semmle.python.security.strings.External
 
 /** Generic taint source from a http request */
-abstract class SimpleHttpRequestTaintSource extends TaintSource {
-
-    override predicate isSourceOf(TaintKind kind) { 
-        kind instanceof ExternalStringKind
-    }
+abstract class HttpRequestTaintSource extends TaintSource {
 
 }
 
@@ -89,3 +85,11 @@ class UntrustedCookie extends TaintKind {
 }
 
 
+/** Generic taint sink in a http response */
+abstract class HttpResponseTaintSink extends TaintSink {
+
+    override predicate sinks(TaintKind kind) { 
+        kind instanceof ExternalStringKind
+    }
+
+}
diff --git a/python/ql/src/semmle/python/web/bottle/Response.qll b/python/ql/src/semmle/python/web/bottle/Response.qll
@@ -22,7 +22,7 @@ private Object theBottleResponseObject() {
     result = theBottleModule().attr("response")
 }
 
-class BottleResponseBodyAssignment extends TaintSink {
+class BottleResponseBodyAssignment extends HttpResponseTaintSink {
 
     BottleResponseBodyAssignment() {
         exists(DefinitionNode lhs |
@@ -37,7 +37,7 @@ class BottleResponseBodyAssignment extends TaintSink {
 
 }
 
-class BottleHandlerFunctionResult extends TaintSink {
+class BottleHandlerFunctionResult extends HttpResponseTaintSink {
 
     BottleHandlerFunctionResult() {
         exists(BottleRoute route, Return ret |
diff --git a/python/ql/src/semmle/python/web/cherrypy/Response.qll b/python/ql/src/semmle/python/web/cherrypy/Response.qll
@@ -7,7 +7,7 @@ import semmle.python.web.cherrypy.General
 
 
 
-class CherryPyExposedFunctionResult extends TaintSink {
+class CherryPyExposedFunctionResult extends HttpResponseTaintSink {
 
     CherryPyExposedFunctionResult() {
         exists(Return ret |
diff --git a/python/ql/src/semmle/python/web/django/Db.qll b/python/ql/src/semmle/python/web/django/Db.qll
@@ -46,7 +46,7 @@ ClassObject theDjangoRawSqlClass() {
  * allows arbitrary SQL statements to be executed, which is a security risk.
  */
 
-class DjangoRawSqlSink extends TaintSink {
+class DjangoRawSqlSink extends SqlInjectionSink {
     DjangoRawSqlSink() {
         exists(CallNode call |
             call = theDjangoRawSqlClass().getACall() and
diff --git a/python/ql/src/semmle/python/web/django/Model.qll b/python/ql/src/semmle/python/web/django/Model.qll
@@ -3,6 +3,7 @@ import python
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Basic
 import semmle.python.web.Http
+import semmle.python.security.injection.Sql
 
 /** A django model class */
 class DjangoModel extends ClassObject {
@@ -68,7 +69,7 @@ class DjangoModelObjects extends TaintSource {
 }
 
 /** A write to a field of a django model, which is a vulnerable to external data. */
-class DjangoModelFieldWrite extends TaintSink {
+class DjangoModelFieldWrite extends SqlInjectionSink {
 
     DjangoModelFieldWrite() {
         exists(AttrNode attr, DjangoModel model |
@@ -111,7 +112,7 @@ class DjangoModelDirectObjectReference extends TaintSink {
  * to be sent to the database, which is a security risk.
  */
 
-class DjangoModelRawCall extends TaintSink {
+class DjangoModelRawCall extends SqlInjectionSink {
 
     DjangoModelRawCall() {
         exists(CallNode raw_call, ControlFlowNode queryset |
@@ -136,7 +137,7 @@ class DjangoModelRawCall extends TaintSink {
  */
 
 
-class DjangoModelExtraCall extends TaintSink {
+class DjangoModelExtraCall extends SqlInjectionSink {
 
     DjangoModelExtraCall() {
         exists(CallNode extra_call, ControlFlowNode queryset |
diff --git a/python/ql/src/semmle/python/web/django/Request.qll b/python/ql/src/semmle/python/web/django/Request.qll
@@ -49,7 +49,7 @@ class DjangoQueryDict extends TaintKind {
 
 }
 
-abstract class DjangoRequestSource extends TaintSource {
+abstract class DjangoRequestSource extends HttpRequestTaintSource {
 
     override string toString() {
         result = "Django request source"
@@ -144,7 +144,7 @@ class UrlRouting extends CallNode {
 }
 
 /** An argument specified in a url routing table */
-class HttpRequestParameter extends TaintSource {
+class HttpRequestParameter extends HttpRequestTaintSource {
 
     HttpRequestParameter() {
         exists(UrlRouting url |
diff --git a/python/ql/src/semmle/python/web/django/Response.qll b/python/ql/src/semmle/python/web/django/Response.qll
@@ -2,6 +2,7 @@ import python
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Basic
 private import semmle.python.web.django.Shared
+private import semmle.python.web.Http
 
 
 /** A django.http.response.Response object
@@ -39,7 +40,7 @@ class DjangoResponseSource extends TaintSource {
 }
 
 /** A write to a django response, which is vulnerable to external data (xss) */
-class DjangoResponseWrite extends TaintSink {
+class DjangoResponseWrite extends HttpResponseTaintSink {
 
     DjangoResponseWrite() {
         exists(AttrNode meth, CallNode call |
@@ -60,7 +61,7 @@ class DjangoResponseWrite extends TaintSink {
 }
 
 /** An argument to initialization of a django response, which is vulnerable to external data (xss) */
-class DjangoResponseContent extends TaintSink {
+class DjangoResponseContent extends HttpResponseTaintSink {
 
     DjangoResponseContent() {
         exists(CallNode call, ClassObject cls |
diff --git a/python/ql/src/semmle/python/web/falcon/Response.qll b/python/ql/src/semmle/python/web/falcon/Response.qll
@@ -30,7 +30,7 @@ class FalconResponseParameter extends TaintSource {
 
 }
 
-class FalconResponseBodySink extends TaintSink {
+class FalconResponseBodySink extends HttpResponseTaintSink {
 
     FalconResponseBodySink() {
         exists(AttrNode attr |
diff --git a/python/ql/src/semmle/python/web/flask/Request.qll b/python/ql/src/semmle/python/web/flask/Request.qll
@@ -16,7 +16,7 @@ private predicate flask_request_attr(AttrNode attr, string name) {
 }
 
 /** Source of external data from a flask request */
-class FlaskRequestData extends SimpleHttpRequestTaintSource {
+class FlaskRequestData extends HttpRequestTaintSource {
 
     FlaskRequestData() {
         not this instanceof FlaskRequestArgs and
@@ -27,14 +27,18 @@ class FlaskRequestData extends SimpleHttpRequestTaintSource {
         )
     }
 
+    override predicate isSourceOf(TaintKind kind) { 
+        kind instanceof ExternalStringKind
+    }
+
     override string toString() {
         result = "flask.request"
     }
 
 }
 
 /** Source of dictionary whose values are externally controlled */
-class FlaskRequestArgs extends TaintSource {
+class FlaskRequestArgs extends HttpRequestTaintSource {
 
     FlaskRequestArgs() {
         exists(string attr |
diff --git a/python/ql/src/semmle/python/web/flask/Response.qll b/python/ql/src/semmle/python/web/flask/Response.qll
@@ -8,7 +8,7 @@ import semmle.python.web.flask.General
 
 /** A flask response, which is vulnerable to any sort of 
  * http response malice. */
-class FlaskRoutedResponse extends TaintSink {
+class FlaskRoutedResponse extends HttpResponseTaintSink {
 
     FlaskRoutedResponse() {
         exists(PyFunctionObject response |
@@ -28,7 +28,7 @@ class FlaskRoutedResponse extends TaintSink {
 }
 
 
-class FlaskResponseArgument extends TaintSink {
+class FlaskResponseArgument extends HttpResponseTaintSink {
 
     FlaskResponseArgument() {
         exists(CallNode call |
diff --git a/python/ql/src/semmle/python/web/pyramid/Response.qll b/python/ql/src/semmle/python/web/pyramid/Response.qll
diff --git a/python/ql/src/semmle/python/web/tornado/Response.qll b/python/ql/src/semmle/python/web/tornado/Response.qll
diff --git a/python/ql/src/semmle/python/web/turbogears/Response.qll b/python/ql/src/semmle/python/web/turbogears/Response.qll
diff --git a/python/ql/src/semmle/python/web/twisted/Response.qll b/python/ql/src/semmle/python/web/twisted/Response.qll

Original file line number	Diff line number	Diff line change
`@@ -1668,3 +1668,4 @@ private predicate sequence_call(ControlFlowNode fromnode, CallNode tonode) {`
`1668`	`1668`	`cls.refersTo(theSetType())`
`1669`	`1669`	`)`
`1670`	`1670`	`}`
	`1671`	`+`