Merge pull request #19053 from owen-mc/go/fp/log-type

Go: Fix false positives when logging using `%T`

Author: owen-mc

go/ql/lib/semmle/go/Concepts.qll

@@ -357,6 +357,23 @@ module RegexpReplaceFunction {
 class LoggerCall extends DataFlow::Node instanceof LoggerCall::Range {
   /** Gets a node that is a part of the logged message. */
   DataFlow::Node getAMessageComponent() { result = super.getAMessageComponent() }
+
+  /**
+   * Gets a node whose value is a part of the logged message.
+   *
+   * Components corresponding to the format specifier "%T" are excluded as
+   * their type is logged rather than their value.
+   */
+  DataFlow::Node getAValueFormattedMessageComponent() {
+    result = this.getAMessageComponent() and
+    not exists(string formatSpecifier |
+      result = this.(StringOps::Formatting::StringFormatCall).getOperand(_, formatSpecifier) and
+      // We already know that `formatSpecifier` starts with `%`, so we check
+      // that it ends with `T` to confirm that it is `%T` or possibly some
+      // variation on it.
+      formatSpecifier.matches("%T")
+    )
+  }
 }
 
 /** Provides a class for modeling new logging APIs. */

go/ql/lib/semmle/go/security/CleartextLoggingCustomizations.qll

@@ -40,7 +40,7 @@ module CleartextLogging {
    * An argument to a logging mechanism.
    */
   class LoggerSink extends Sink {
-    LoggerSink() { this = any(LoggerCall log).getAMessageComponent() }
+    LoggerSink() { this = any(LoggerCall log).getAValueFormattedMessageComponent() }
   }
 
   /**

go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll

@@ -35,7 +35,7 @@ module LogInjection {
 
   /** An argument to a logging mechanism. */
   class LoggerSink extends Sink {
-    LoggerSink() { this = any(LoggerCall log).getAMessageComponent() }
+    LoggerSink() { this = any(LoggerCall log).getAValueFormattedMessageComponent() }
   }
 
   /**

go/ql/src/Security/CWE-352/ConstantOauth2State.ql

@@ -138,7 +138,9 @@ predicate privateUrlFlowsToAuthCodeUrlCall(DataFlow::CallNode call) {
 
 module FlowToPrintConfig implements DataFlow::ConfigSig {
   additional predicate isSinkCall(DataFlow::Node sink, DataFlow::CallNode call) {
-    exists(LoggerCall logCall | call = logCall | sink = logCall.getAMessageComponent())
+    exists(LoggerCall logCall | call = logCall |
+      sink = logCall.getAValueFormattedMessageComponent()
+    )
   }
 
   predicate isSource(DataFlow::Node source) { source = any(AuthCodeUrl m).getACall().getResult() }

go/ql/src/change-notes/2025-03-20-logging-false-positive-type-format-specifier.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* False positives in "Log entries created from user input" (`go/log-injection`) and "Clear-text logging of sensitive information" (`go/clear-text-logging`) which involved the verb `%T` in a format specifier have been fixed. As a result, some users may also see more alerts from the "Use of constant `state` value in OAuth 2.0 URL" (`go/constant-oauth2-state`) query.

go/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.ql

@@ -4,14 +4,20 @@ import ModelValidation
 import utils.test.InlineExpectationsTest
 
 module LoggerTest implements TestSig {
-  string getARelevantTag() { result = "logger" }
+  string getARelevantTag() { result = ["type-logger", "logger"] }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(LoggerCall log |
       log.getLocation() = location and
       element = log.toString() and
-      value = log.getAMessageComponent().toString() and
-      tag = "logger"
+      (
+        value = log.getAValueFormattedMessageComponent().toString() and
+        tag = "logger"
+        or
+        value = log.getAMessageComponent().toString() and
+        not value = log.getAValueFormattedMessageComponent().toString() and
+        tag = "type-logger"
+      )
     )
   }
 }

go/ql/test/library-tests/semmle/go/concepts/LoggerCall/glog.go

@@ -30,6 +30,13 @@ func glogTest() {
 	glog.Warningf(fmt, text)   // $ logger=fmt logger=text
 	glog.Warningln(text)       // $ logger=text
 
+	// components corresponding to the format specifier "%T" are not considered vulnerable
+	glog.Errorf("%s: found type %T", text, v)   // $ logger="%s: found type %T" logger=text type-logger=v
+	glog.Exitf("%s: found type %T", text, v)    // $ logger="%s: found type %T" logger=text type-logger=v
+	glog.Fatalf("%s: found type %T", text, v)   // $ logger="%s: found type %T" logger=text type-logger=v
+	glog.Infof("%s: found type %T", text, v)    // $ logger="%s: found type %T" logger=text type-logger=v
+	glog.Warningf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+
 	klog.Error(text)           // $ logger=text
 	klog.ErrorDepth(0, text)   // $ logger=text
 	klog.Errorf(fmt, text)     // $ logger=fmt logger=text
@@ -50,4 +57,11 @@ func glogTest() {
 	klog.WarningDepth(0, text) // $ logger=text
 	klog.Warningf(fmt, text)   // $ logger=fmt logger=text
 	klog.Warningln(text)       // $ logger=text
+
+	// components corresponding to the format specifier "%T" are not considered vulnerable
+	klog.Errorf("%s: found type %T", text, v)   // $ logger="%s: found type %T" logger=text type-logger=v
+	klog.Exitf("%s: found type %T", text, v)    // $ logger="%s: found type %T" logger=text type-logger=v
+	klog.Fatalf("%s: found type %T", text, v)   // $ logger="%s: found type %T" logger=text type-logger=v
+	klog.Infof("%s: found type %T", text, v)    // $ logger="%s: found type %T" logger=text type-logger=v
+	klog.Warningf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
 }

go/ql/test/library-tests/semmle/go/concepts/LoggerCall/logrus.go

@@ -32,4 +32,8 @@ func logrusCalls() {
 	logrus.Panicln(text)     // $ logger=text
 	logrus.Infof(fmt, text)  // $ logger=fmt logger=text
 	logrus.FatalFn(fn)       // $ logger=fn
+
+	// components corresponding to the format specifier "%T" are not considered vulnerable
+	logrus.Infof("%s: found type %T", text, v)  // $ logger="%s: found type %T" logger=text type-logger=v
+	logrus.Fatalf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
 }

go/ql/test/library-tests/semmle/go/concepts/LoggerCall/main.go

@@ -3,6 +3,8 @@ package main
 const fmt = "formatted %s string"
 const text = "test"
 
-func main() {
+var v []byte
 
+func main() {
+	stdlib()
 }

go/ql/test/library-tests/semmle/go/concepts/LoggerCall/stdlib.go

@@ -17,6 +17,11 @@ func stdlib() {
 	logger.Printf(fmt, text) // $ logger=fmt logger=text
 	logger.Println(text)     // $ logger=text
 
+	// components corresponding to the format specifier "%T" are not considered vulnerable
+	logger.Fatalf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+	logger.Panicf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+	logger.Printf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+
 	log.SetPrefix("prefix: ")
 	log.Fatal(text)       // $ logger=text
 	log.Fatalf(fmt, text) // $ logger=fmt logger=text
@@ -27,4 +32,9 @@ func stdlib() {
 	log.Print(text)       // $ logger=text
 	log.Printf(fmt, text) // $ logger=fmt logger=text
 	log.Println(text)     // $ logger=text
+
+	// components corresponding to the format specifier "%T" are not considered vulnerable
+	log.Fatalf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+	log.Panicf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
+	log.Printf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text type-logger=v
 }