Merge pull request #14853 from geoffw0/logsinks

Swift: More sinks for swift/cleartext-logging

Author: geoffw0

swift/ql/lib/codeql/swift/StringFormat.qll

@@ -78,10 +78,13 @@ class NsLog extends FormattingFunction, FreeFunction {
 }
 
 /**
- * The `NSException.raise` method.
+ * The `NSException.init` and `NSException.raise` methods.
  */
 class NsExceptionRaise extends FormattingFunction, Method {
-  NsExceptionRaise() { this.hasQualifiedName("NSException", "raise(_:format:arguments:)") }
+  NsExceptionRaise() {
+    this.hasQualifiedName("NSException", "init(name:reason:userInfo:)") or
+    this.hasQualifiedName("NSException", "raise(_:format:arguments:)")
+  }
 
   override int getFormatParameterIndex() { result = 1 }
 }
@@ -91,10 +94,17 @@ class NsExceptionRaise extends FormattingFunction, Method {
  */
 class PrintfFormat extends FormattingFunction, FreeFunction {
   int formatParamIndex;
+  string modeChars;
 
   PrintfFormat() {
-    this.getShortName().matches("%printf%") and this.getParam(formatParamIndex).getName() = "format"
+    modeChars = this.getShortName().regexpCapture("(.*)printf.*", 1) and
+    this.getParam(formatParamIndex).getName() = "format"
   }
 
   override int getFormatParameterIndex() { result = formatParamIndex }
+
+  /**
+   * Holds if this `printf` is a variant of `sprintf`.
+   */
+  predicate isSprintf() { modeChars.charAt(_) = "s" }
 }

swift/ql/lib/codeql/swift/dataflow/internal/TaintTrackingPrivate.qll

@@ -61,6 +61,11 @@ private module Cached {
       se = nodeTo.asExpr()
     )
     or
+    // flow through autoclosure expressions (which turn value arguments into closure arguments);
+    // if the value is tainted, it's helpful to consider the autoclosure itself to be tainted as
+    // well for the purposes of matching sink models.
+    nodeFrom.asExpr() = nodeTo.asExpr().(AutoClosureExpr).getExpr()
+    or
     // flow through the read of a content that inherits taint
     exists(DataFlow::ContentSet f |
       readStep(nodeFrom, f, nodeTo) and

swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll

@@ -4,6 +4,7 @@ import swift
 private import codeql.swift.dataflow.DataFlow
 private import codeql.swift.dataflow.ExternalFlow
 private import codeql.swift.security.SensitiveExprs
+private import codeql.swift.StringFormat
 
 /** A data flow sink for cleartext logging of sensitive data vulnerabilities. */
 abstract class CleartextLoggingSink extends DataFlow::Node { }
@@ -93,6 +94,48 @@ private class CleartextLoggingFieldAdditionalFlowStep extends CleartextLoggingAd
   }
 }
 
+/**
+ * A sink that appears to be an imported C `printf` variant.
+ */
+private class PrintfCleartextLoggingSink extends CleartextLoggingSink {
+  PrintfCleartextLoggingSink() {
+    exists(CallExpr ce, PrintfFormat f |
+      ce.getStaticTarget() = f and
+      (
+        this.asExpr() = ce.getArgument(f.getFormatParameterIndex()).getExpr() or
+        this.asExpr() = ce.getArgument(f.getNumberOfParams() - 1).getExpr()
+      ) and
+      not f.isSprintf()
+    )
+  }
+}
+
+/**
+ * Holds if `label` looks like the name of a logging function.
+ */
+bindingset[label]
+private predicate logLikeHeuristic(string label) {
+  label.regexpMatch("(l|.*L)og([A-Z0-9].*)?") // e.g. "logMessage", "debugLog"
+}
+
+/**
+ * A cleartext logging sink that is determined by imprecise methods.
+ */
+class HeuristicCleartextLoggingSink extends CleartextLoggingSink {
+  HeuristicCleartextLoggingSink() {
+    exists(CallExpr ce, Function f, Expr e |
+      (
+        logLikeHeuristic(f.getShortName()) or
+        logLikeHeuristic(f.getDeclaringDecl().(NominalTypeDecl).getName())
+      ) and
+      ce.getStaticTarget() = f and
+      ce.getAnArgument().getExpr() = e and
+      e.getType().getUnderlyingType().getName() = ["String", "NSString"] and
+      this.asExpr() = e
+    )
+  }
+}
+
 private class LoggingSinks extends SinkModelCsv {
   override predicate row(string row) {
     row =
@@ -123,6 +166,8 @@ private class LoggingSinks extends SinkModelCsv {
         ";;false;os_log(_:log:_:);;;Argument[2];log-injection",
         ";;false;os_log(_:dso:log:_:_:);;;Argument[0,4];log-injection",
         ";;false;os_log(_:dso:log:type:_:);;;Argument[0,4];log-injection",
+        ";NSException;true;init(name:reason:userInfo:);;;Argument[1];log-injection",
+        ";NSException;true;raise(_:format:arguments:);;;Argument[1..2];log-injection",
       ]
   }
 }

swift/ql/src/change-notes/2023-11-20-cleartext-logging.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added additional sinks for the "Cleartext logging of sensitive information" (`swift/cleartext-logging`) query. Some of these sinks are heuristic (imprecise) in nature.

swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql

@@ -7,11 +7,10 @@ module CleartextLogging implements TestSig {
   string getARelevantTag() { result = "hasCleartextLogging" }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(DataFlow::Node source, DataFlow::Node sink, Expr sinkExpr |
+    exists(DataFlow::Node source, DataFlow::Node sink |
       CleartextLoggingFlow::flow(source, sink) and
-      sinkExpr = sink.asExpr() and
-      location = sinkExpr.getLocation() and
-      element = sinkExpr.toString() and
+      location = sink.getLocation() and
+      element = sink.toString() and
       tag = "hasCleartextLogging" and
       value = source.asExpr().getLocation().getStartLine().toString()
     )