Python: Fix results outside DB for CleartextLogging

Author: RasmusWL

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

@@ -57,16 +57,43 @@ module CleartextLogging {
   /** A piece of data printed, considered as a flow sink. */
   class PrintedDataAsSink extends Sink {
     PrintedDataAsSink() {
-      this = API::builtin("print").getACall().getArg(_)
-      or
-      // special handling of writing to `sys.stdout` and `sys.stderr`, which is
-      // essentially the same as printing
-      this =
-        API::moduleImport("sys")
-            .getMember(["stdout", "stderr"])
-            .getMember("write")
-            .getACall()
-            .getArg(0)
+      (
+        this = API::builtin("print").getACall().getArg(_)
+        or
+        // special handling of writing to `sys.stdout` and `sys.stderr`, which is
+        // essentially the same as printing
+        this =
+          API::moduleImport("sys")
+              .getMember(["stdout", "stderr"])
+              .getMember("write")
+              .getACall()
+              .getArg(0)
+      ) and
+      // since some of the inner error handling implementation of the logging module is
+      // ```py
+      //         sys.stderr.write('Message: %r\n'
+      //         'Arguments: %s\n' % (record.msg,
+      //                              record.args))
+      // ```
+      // any time we would report flow to such a logging sink, we can ALSO report
+      // the flow to the `record.msg`/`record.args` sinks -- obviously we
+      // don't want that.
+      //
+      // However, simply removing taint edges out of a sink is not a good enough solution,
+      // since we would only flag one of the `logging.info` calls in the following example
+      // due to use-use flow
+      // ```py
+      // logging.info(user_controlled)
+      // logging.info(user_controlled)
+      // ```
+      //
+      // The same approach is used in the command injection query.
+      not exists(Module loggingInit |
+        loggingInit.getName() = "logging.__init__" and
+        this.getScope().getEnclosingModule() = loggingInit and
+        // do allow this call if we're analyzing logging/__init__.py as part of CPython though
+        not exists(loggingInit.getFile().getRelativePath())
+      )
     }
   }
 }

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

@@ -77,7 +77,8 @@ module CommandInjection {
       // https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/os.py#L974
       // https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
       //
-      // The same approach is used in the path-injection and cleartext-storage queries.
+      // The same approach is used in the path-injection, cleartext-storage, and
+      // cleartext-logging queries.
       not this.getScope().getEnclosingModule().getName() in [
           "os", "subprocess", "platform", "popen2"
         ]