python: move log injection out of experimental

- move from custom concept `LogOutput` to standard concept `Logging` - remove `Log.qll` from experimental frameworks - fold models into standard models (naively for now) - stdlib: - make Logger module public - broaden definition of instance - add `extra` keyword as possible source - flak: add app.logger as logger instance - django: `add django.utils.log.request_logger` as logger instance (should we add the rest?) - remove LogOutput from experimental concepts
github · RasmusWL · Feb 23, 2022 · Jan 25, 2022 · Jan 25, 2022 · Jan 25, 2022
commit 20d54543fd9e45012b970e30f1de6d6d32c9aef8
@@ -2295,4 +2295,17 @@ module PrivateDjango {
 
     override string getMimetypeDefault() { none() }
   }
+
+  // ---------------------------------------------------------------------------
+  // Logging
+  // ---------------------------------------------------------------------------
+  /**
+   * Django provides a standard Python logger.
+   */
+  private class DjangoLogger extends Stdlib::Logger::LoggerInstance {
+    DjangoLogger() {
+      this =
+        API::moduleImport("django").getMember("utils").getMember("log").getMember("request_logger")
+    }
+  }
 }
@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import semmle.python.Concepts
 private import semmle.python.frameworks.Werkzeug
+private import semmle.python.frameworks.Stdlib
 private import semmle.python.ApiGraphs
 private import semmle.python.frameworks.internal.InstanceTaintStepsHelper
 private import semmle.python.security.dataflow.PathInjectionCustomizations
@@ -569,4 +570,14 @@ module Flask {
       result in [this.getArg(0), this.getArgByName("filename_or_fp")]
     }
   }
+
+  // ---------------------------------------------------------------------------
+  // Logging
+  // ---------------------------------------------------------------------------
+  /**
+   * The attribute `logger` on a Flask application is a standard Python logger.
+   */
+  private class FlaskLogger extends Stdlib::Logger::LoggerInstance {
+    FlaskLogger() { this = FlaskApp::instance().getMember("logger") }
+  }
 }
@@ -237,6 +237,42 @@ module Stdlib {
       }
     }
   }
+
+  // ---------------------------------------------------------------------------
+  // logging
+  // ---------------------------------------------------------------------------
+  /**
+   * Provides models for the `logging.Logger` class and subclasses.
+   *
+   * See https://docs.python.org/3.9/library/logging.html#logging.Logger.
+   */
+  module Logger {
+    /**
+     * An instance of `logging.Logger`. Extend this class to model new instances.
+     * Most major frameworks will provide a logger instance as a class attribute.
+     */
+    abstract class LoggerInstance extends API::Node {
+      override string toString() { result = "logger" }
+    }
+
+    /** Gets a reference to the `logging.Logger` class or any subclass. */
+    API::Node subclassRef() {
+      result = API::moduleImport("logging").getMember("Logger").getASubclass*()
+    }
+
+    /** Gets a reference to an instance of `logging.Logger` or any subclass. */
+    API::Node instance() {
+      result instanceof LoggerInstance
+      or
+      result = subclassRef().getReturn()
+      or
+      result = API::moduleImport("logging")
+      or
+      result = API::moduleImport("logging").getMember("root")
+      or
+      result = API::moduleImport("logging").getMember("getLogger").getReturn()
+    }
+  }
 }
 
 /**
@@ -2642,27 +2678,6 @@ private module StdlibPrivate {
   // ---------------------------------------------------------------------------
   // logging
   // ---------------------------------------------------------------------------
-  /**
-   * Provides models for the `logging.Logger` class and subclasses.
-   *
-   * See https://docs.python.org/3.9/library/logging.html#logging.Logger.
-   */
-  module Logger {
-    /** Gets a reference to the `logging.Logger` class or any subclass. */
-    API::Node subclassRef() {
-      result = API::moduleImport("logging").getMember("Logger").getASubclass*()
-    }
-
-    /** Gets a reference to an instance of `logging.Logger` or any subclass. */
-    API::Node instance() {
-      result = subclassRef().getReturn()
-      or
-      result = API::moduleImport("logging").getMember("root")
-      or
-      result = API::moduleImport("logging").getMember("getLogger").getReturn()
-    }
-  }
-
   /**
    * A call to one of the logging methods from `logging` or on a `logging.Logger`
    * subclass.
@@ -2683,14 +2698,12 @@ private module StdlibPrivate {
         method = "log" and
         msgIndex = 1
       |
-        this = Logger::instance().getMember(method).getACall()
-        or
-        this = API::moduleImport("logging").getMember(method).getACall()
+        this = Stdlib::Logger::instance().getMember(method).getACall()
       )
     }
 
     override DataFlow::Node getAnInput() {
-      result = this.getArgByName("msg")
+      result = this.getArgByName(["msg", "extra"])
       or
       result = this.getArg(any(int i | i >= msgIndex))
     }

@@ -1,6 +1,6 @@
 import python
 import semmle.python.Concepts
-import experimental.semmle.python.Concepts
+import semmle.python.Concepts
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.dataflow.new.RemoteFlowSources
@@ -13,7 +13,7 @@ class LogInjectionFlowConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSink(DataFlow::Node sink) { sink = any(LogOutput logoutput).getAnInput() }
+  override predicate isSink(DataFlow::Node sink) { sink = any(Logging write).getAnInput() }
 
   override predicate isSanitizer(DataFlow::Node node) {
     exists(CallNode call |

@@ -11,7 +11,7 @@
  */
 
 import python
-import experimental.semmle.python.security.injection.LogInjection
+import semmle.python.security.dataflow.LogInjection
 import DataFlow::PathGraph
 
 from LogInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink

@@ -14,36 +14,6 @@ private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
 
-/** Provides classes for modeling log related APIs. */
-module LogOutput {
-  /**
-   * A data flow node for log output.
-   *
-   * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `LogOutput` instead.
-   */
-  abstract class Range extends DataFlow::Node {
-    /**
-     * Get the parameter value of the log output function.
-     */
-    abstract DataFlow::Node getAnInput();
-  }
-}
-
-/**
- * A data flow node for log output.
- *
- * Extend this class to refine existing API models. If you want to model new APIs,
- * extend `LogOutput::Range` instead.
- */
-class LogOutput extends DataFlow::Node {
-  LogOutput::Range range;
-
-  LogOutput() { this = range }
-
-  DataFlow::Node getAnInput() { result = range.getAnInput() }
-}
-
 /** Provides classes for modeling LDAP query execution-related APIs. */
 module LDAPQuery {
   /**

@@ -8,7 +8,6 @@ private import experimental.semmle.python.frameworks.Django
 private import experimental.semmle.python.frameworks.Werkzeug
 private import experimental.semmle.python.frameworks.LDAP
 private import experimental.semmle.python.frameworks.NoSQL
-private import experimental.semmle.python.frameworks.Log
 private import experimental.semmle.python.frameworks.JWT
 private import experimental.semmle.python.libraries.PyJWT
 private import experimental.semmle.python.libraries.Authlib

@@ -0,0 +1 @@
+Security/CWE-117/LogInjection.ql