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

python/ql/lib/semmle/python/frameworks/Django.qll

@@ -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")
+    }
+  }
 }

python/ql/lib/semmle/python/frameworks/Flask.qll

@@ -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") }
+  }
 }

python/ql/lib/semmle/python/frameworks/Stdlib.qll

@@ -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))
     }

python/ql/lib/semmle/python/security/dataflow/LogInjection.qll

@@ -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 |

python/ql/src/Security/CWE-117/LogInjection.ql

@@ -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

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -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 {
   /**

python/ql/src/experimental/semmle/python/Frameworks.qll

@@ -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

python/ql/src/experimental/semmle/python/frameworks/Log.qll

@@ -1,118 +0,0 @@
-/**
- * Provides classes modeling security-relevant aspects of the log libraries.
- */
-
-private import python
-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.frameworks.Flask
-private import semmle.python.ApiGraphs
-
-/**
- * Provides models for Python's log-related libraries.
- */
-private module log {
-  /**
-   * Log output method list.
-   *
-   * See https://docs.python.org/3/library/logging.html#logger-objects
-   */
-  private class LogOutputMethods extends string {
-    LogOutputMethods() {
-      this in ["info", "error", "warn", "warning", "debug", "critical", "exception", "log"]
-    }
-  }
-
-  /**
-   * The class used to find the log output method of the `logging` module.
-   *
-   * See `LogOutputMethods`
-   */
-  private class LoggingCall extends DataFlow::CallCfgNode, LogOutput::Range {
-    LoggingCall() {
-      this = API::moduleImport("logging").getMember(any(LogOutputMethods m)).getACall()
-    }
-
-    override DataFlow::Node getAnInput() {
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() != "log" and
-      result in [this.getArg(_), this.getArgByName(_)] // this includes the arg named "msg"
-      or
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "log" and
-      result in [this.getArg(any(int i | i > 0)), this.getArgByName(any(string s | s != "level"))]
-    }
-  }
-
-  /**
-   * The class used to find log output methods related to the `logging.getLogger` instance.
-   *
-   * See `LogOutputMethods`
-   */
-  private class LoggerCall extends DataFlow::CallCfgNode, LogOutput::Range {
-    LoggerCall() {
-      this =
-        API::moduleImport("logging")
-            .getMember("getLogger")
-            .getReturn()
-            .getMember(any(LogOutputMethods m))
-            .getACall()
-    }
-
-    override DataFlow::Node getAnInput() {
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() != "log" and
-      result in [this.getArg(_), this.getArgByName(_)] // this includes the arg named "msg"
-      or
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "log" and
-      result in [this.getArg(any(int i | i > 0)), this.getArgByName(any(string s | s != "level"))]
-    }
-  }
-
-  /**
-   * The class used to find the relevant log output method of the `flask.Flask.logger` instance (flask application).
-   *
-   * See `LogOutputMethods`
-   */
-  private class FlaskLoggingCall extends DataFlow::CallCfgNode, LogOutput::Range {
-    FlaskLoggingCall() {
-      this =
-        Flask::FlaskApp::instance()
-            .getMember("logger")
-            .getMember(any(LogOutputMethods m))
-            .getACall()
-    }
-
-    override DataFlow::Node getAnInput() {
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() != "log" and
-      result in [this.getArg(_), this.getArgByName(_)] // this includes the arg named "msg"
-      or
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "log" and
-      result in [this.getArg(any(int i | i > 0)), this.getArgByName(any(string s | s != "level"))]
-    }
-  }
-
-  /**
-   * The class used to find the relevant log output method of the `django.utils.log.request_logger` instance (django application).
-   *
-   * See `LogOutputMethods`
-   */
-  private class DjangoLoggingCall extends DataFlow::CallCfgNode, LogOutput::Range {
-    DjangoLoggingCall() {
-      this =
-        API::moduleImport("django")
-            .getMember("utils")
-            .getMember("log")
-            .getMember("request_logger")
-            .getMember(any(LogOutputMethods m))
-            .getACall()
-    }
-
-    override DataFlow::Node getAnInput() {
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() != "log" and
-      result in [this.getArg(_), this.getArgByName(_)] // this includes the arg named "msg"
-      or
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "log" and
-      result in [this.getArg(any(int i | i > 0)), this.getArgByName(any(string s | s != "level"))]
-    }
-  }
-}

python/ql/test/experimental/query-tests/Security/CWE-117/LogInjection.qlref

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

python/ql/test/query-tests/Security/CWE-117-LogInjection/LogInjection.qlref

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