Merge pull request #15268 from atorralba/atorralba/go/cleartext-loggi…

…ng-src-and-sink-improvs Go: Adds sources and sinks to `go/clear-text-logging`
github · Jan 10, 2024 · 52d3e3d · 52d3e3d
2 parents 3534bfc + 5e8c63c
commit 52d3e3d
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 21 deletions.
diff --git a/go/ql/lib/semmle/go/frameworks/stdlib/Log.qll b/go/ql/lib/semmle/go/frameworks/stdlib/Log.qll
@@ -7,13 +7,21 @@ import go
 /** Provides models of commonly used functions in the `log` package. */
 module Log {
   private class LogFunction extends Function {
+    int firstPrintedArg;
+
     LogFunction() {
-      exists(string fn | fn.matches(["Fatal%", "Panic%", "Print%"]) |
+      exists(string fn |
+        fn.matches(["Fatal%", "Panic%", "Print%"]) and firstPrintedArg = 0
+        or
+        fn = "Output" and firstPrintedArg = 1
+      |
         this.hasQualifiedName("log", fn)
         or
         this.(Method).hasQualifiedName("log", "Logger", fn)
       )
     }
+
+    int getFirstPrintedArg() { result = firstPrintedArg }
   }
 
   private class LogFormatter extends StringOps::Formatting::Range instanceof LogFunction {
@@ -23,9 +31,13 @@ module Log {
   }
 
   private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
-    LogCall() { this = any(LogFunction f).getACall() }
+    LogFunction target;
 
-    override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
+    LogCall() { this = target.getACall() }
+
+    override DataFlow::Node getAMessageComponent() {
+      result = this.getSyntacticArgument(any(int i | i >= target.getFirstPrintedArg()))
+    }
   }
 
   /** A fatal log function, which calls `os.Exit`. */

diff --git a/go/ql/lib/semmle/go/security/CleartextLogging.qll b/go/ql/lib/semmle/go/security/CleartextLogging.qll
@@ -74,6 +74,8 @@ module CleartextLogging {
       )
     }
 
+    predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
+
     predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
       // A taint propagating data-flow edge through structs: a tainted write taints the entire struct.
       exists(Write write |

diff --git a/go/ql/lib/semmle/go/security/SensitiveActions.qll b/go/ql/lib/semmle/go/security/SensitiveActions.qll
@@ -35,7 +35,7 @@ module HeuristicNames {
    */
   string maybePassword() {
     result = "(?is).*pass(wd|word|code|phrase)(?!.*question).*" or
-    result = "(?is).*(auth(entication|ori[sz]ation)?|api)key.*"
+    result = "(?is).*(auth(entication|ori[sz]ation)?|api|secret)key.*"
   }
 
   /**

diff --git a/go/ql/src/change-notes/2024-01-09-cleartext-logging-new-sources-and-sinks.md b/go/ql/src/change-notes/2024-01-09-cleartext-logging-new-sources-and-sinks.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added more sources and sinks to the query `go/clear-text-logging`.
diff --git a/go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected b/go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected
@@ -6,8 +6,6 @@ edges
 | klog.go:21:11:21:16 | definition of header | klog.go:22:15:22:20 | header |
 | klog.go:21:27:21:33 | headers | klog.go:21:4:24:4 | range statement[1] |
 | klog.go:28:13:28:20 | selection of Header | klog.go:28:13:28:41 | call to Get |
-| main.go:20:2:20:7 | definition of fields | main.go:22:29:22:34 | fields |
-| main.go:21:19:21:26 | password | main.go:20:2:20:7 | definition of fields |
 | overrides.go:9:9:9:16 | password | overrides.go:13:14:13:23 | call to String |
 | passwords.go:8:12:8:12 | definition of x | passwords.go:9:14:9:14 | x |
 | passwords.go:30:8:30:15 | password | passwords.go:8:12:8:12 | definition of x |
@@ -53,7 +51,6 @@ edges
 | protos/query/query.pb.go:117:7:117:7 | definition of x [pointer, Description] | protos/query/query.pb.go:119:10:119:10 | x [pointer, Description] |
 | protos/query/query.pb.go:119:10:119:10 | implicit dereference [Description] | protos/query/query.pb.go:119:10:119:22 | selection of Description |
 | protos/query/query.pb.go:119:10:119:10 | x [pointer, Description] | protos/query/query.pb.go:119:10:119:10 | implicit dereference [Description] |
-| util.go:16:9:16:18 | selection of password | passwords.go:28:14:28:28 | call to getPassword |
 nodes
 | klog.go:20:3:25:3 | range statement[1] | semmle.label | range statement[1] |
 | klog.go:20:13:20:19 | definition of headers | semmle.label | definition of headers |
@@ -64,13 +61,35 @@ nodes
 | klog.go:22:15:22:20 | header | semmle.label | header |
 | klog.go:28:13:28:20 | selection of Header | semmle.label | selection of Header |
 | klog.go:28:13:28:41 | call to Get | semmle.label | call to Get |
-| main.go:15:14:15:21 | password | semmle.label | password |
-| main.go:17:12:17:19 | password | semmle.label | password |
-| main.go:18:17:18:24 | password | semmle.label | password |
-| main.go:20:2:20:7 | definition of fields | semmle.label | definition of fields |
-| main.go:21:19:21:26 | password | semmle.label | password |
-| main.go:22:29:22:34 | fields | semmle.label | fields |
-| main.go:25:35:25:42 | password | semmle.label | password |
+| main.go:15:12:15:19 | password | semmle.label | password |
+| main.go:16:17:16:24 | password | semmle.label | password |
+| main.go:17:13:17:20 | password | semmle.label | password |
+| main.go:18:14:18:21 | password | semmle.label | password |
+| main.go:19:12:19:19 | password | semmle.label | password |
+| main.go:20:17:20:24 | password | semmle.label | password |
+| main.go:21:13:21:20 | password | semmle.label | password |
+| main.go:22:14:22:21 | password | semmle.label | password |
+| main.go:23:12:23:19 | password | semmle.label | password |
+| main.go:24:17:24:24 | password | semmle.label | password |
+| main.go:25:13:25:20 | password | semmle.label | password |
+| main.go:26:14:26:21 | password | semmle.label | password |
+| main.go:27:16:27:23 | password | semmle.label | password |
+| main.go:30:10:30:17 | password | semmle.label | password |
+| main.go:31:15:31:22 | password | semmle.label | password |
+| main.go:32:11:32:18 | password | semmle.label | password |
+| main.go:33:12:33:19 | password | semmle.label | password |
+| main.go:34:10:34:17 | password | semmle.label | password |
+| main.go:35:15:35:22 | password | semmle.label | password |
+| main.go:36:11:36:18 | password | semmle.label | password |
+| main.go:37:12:37:19 | password | semmle.label | password |
+| main.go:38:10:38:17 | password | semmle.label | password |
+| main.go:39:15:39:22 | password | semmle.label | password |
+| main.go:40:11:40:18 | password | semmle.label | password |
+| main.go:41:12:41:19 | password | semmle.label | password |
+| main.go:42:14:42:21 | password | semmle.label | password |
+| main.go:44:12:44:19 | password | semmle.label | password |
+| main.go:45:17:45:24 | password | semmle.label | password |
+| main.go:52:35:52:42 | password | semmle.label | password |
 | overrides.go:9:9:9:16 | password | semmle.label | password |
 | overrides.go:13:14:13:23 | call to String | semmle.label | call to String |
 | passwords.go:8:12:8:12 | definition of x | semmle.label | definition of x |
@@ -135,24 +154,46 @@ nodes
 | protos/query/query.pb.go:119:10:119:10 | implicit dereference [Description] | semmle.label | implicit dereference [Description] |
 | protos/query/query.pb.go:119:10:119:10 | x [pointer, Description] | semmle.label | x [pointer, Description] |
 | protos/query/query.pb.go:119:10:119:22 | selection of Description | semmle.label | selection of Description |
-| util.go:16:9:16:18 | selection of password | semmle.label | selection of password |
 subpaths
 | protobuf.go:14:14:14:18 | query [pointer, Description] | protos/query/query.pb.go:117:7:117:7 | definition of x [pointer, Description] | protos/query/query.pb.go:119:10:119:22 | selection of Description | protobuf.go:14:14:14:35 | call to GetDescription |
 #select
 | klog.go:22:15:22:20 | header | klog.go:20:30:20:37 | selection of Header | klog.go:22:15:22:20 | header | $@ flows to a logging call. | klog.go:20:30:20:37 | selection of Header | Sensitive data returned by HTTP request headers |
 | klog.go:28:13:28:41 | call to Get | klog.go:28:13:28:20 | selection of Header | klog.go:28:13:28:41 | call to Get | $@ flows to a logging call. | klog.go:28:13:28:20 | selection of Header | Sensitive data returned by HTTP request headers |
-| main.go:15:14:15:21 | password | main.go:15:14:15:21 | password | main.go:15:14:15:21 | password | $@ flows to a logging call. | main.go:15:14:15:21 | password | Sensitive data returned by an access to password |
-| main.go:17:12:17:19 | password | main.go:17:12:17:19 | password | main.go:17:12:17:19 | password | $@ flows to a logging call. | main.go:17:12:17:19 | password | Sensitive data returned by an access to password |
-| main.go:18:17:18:24 | password | main.go:18:17:18:24 | password | main.go:18:17:18:24 | password | $@ flows to a logging call. | main.go:18:17:18:24 | password | Sensitive data returned by an access to password |
-| main.go:22:29:22:34 | fields | main.go:21:19:21:26 | password | main.go:22:29:22:34 | fields | $@ flows to a logging call. | main.go:21:19:21:26 | password | Sensitive data returned by an access to password |
-| main.go:25:35:25:42 | password | main.go:25:35:25:42 | password | main.go:25:35:25:42 | password | $@ flows to a logging call. | main.go:25:35:25:42 | password | Sensitive data returned by an access to password |
+| main.go:15:12:15:19 | password | main.go:15:12:15:19 | password | main.go:15:12:15:19 | password | $@ flows to a logging call. | main.go:15:12:15:19 | password | Sensitive data returned by an access to password |
+| main.go:16:17:16:24 | password | main.go:16:17:16:24 | password | main.go:16:17:16:24 | password | $@ flows to a logging call. | main.go:16:17:16:24 | password | Sensitive data returned by an access to password |
+| main.go:17:13:17:20 | password | main.go:17:13:17:20 | password | main.go:17:13:17:20 | password | $@ flows to a logging call. | main.go:17:13:17:20 | password | Sensitive data returned by an access to password |
+| main.go:18:14:18:21 | password | main.go:18:14:18:21 | password | main.go:18:14:18:21 | password | $@ flows to a logging call. | main.go:18:14:18:21 | password | Sensitive data returned by an access to password |
+| main.go:19:12:19:19 | password | main.go:19:12:19:19 | password | main.go:19:12:19:19 | password | $@ flows to a logging call. | main.go:19:12:19:19 | password | Sensitive data returned by an access to password |
+| main.go:20:17:20:24 | password | main.go:20:17:20:24 | password | main.go:20:17:20:24 | password | $@ flows to a logging call. | main.go:20:17:20:24 | password | Sensitive data returned by an access to password |
+| main.go:21:13:21:20 | password | main.go:21:13:21:20 | password | main.go:21:13:21:20 | password | $@ flows to a logging call. | main.go:21:13:21:20 | password | Sensitive data returned by an access to password |
+| main.go:22:14:22:21 | password | main.go:22:14:22:21 | password | main.go:22:14:22:21 | password | $@ flows to a logging call. | main.go:22:14:22:21 | password | Sensitive data returned by an access to password |
+| main.go:23:12:23:19 | password | main.go:23:12:23:19 | password | main.go:23:12:23:19 | password | $@ flows to a logging call. | main.go:23:12:23:19 | password | Sensitive data returned by an access to password |
+| main.go:24:17:24:24 | password | main.go:24:17:24:24 | password | main.go:24:17:24:24 | password | $@ flows to a logging call. | main.go:24:17:24:24 | password | Sensitive data returned by an access to password |
+| main.go:25:13:25:20 | password | main.go:25:13:25:20 | password | main.go:25:13:25:20 | password | $@ flows to a logging call. | main.go:25:13:25:20 | password | Sensitive data returned by an access to password |
+| main.go:26:14:26:21 | password | main.go:26:14:26:21 | password | main.go:26:14:26:21 | password | $@ flows to a logging call. | main.go:26:14:26:21 | password | Sensitive data returned by an access to password |
+| main.go:27:16:27:23 | password | main.go:27:16:27:23 | password | main.go:27:16:27:23 | password | $@ flows to a logging call. | main.go:27:16:27:23 | password | Sensitive data returned by an access to password |
+| main.go:30:10:30:17 | password | main.go:30:10:30:17 | password | main.go:30:10:30:17 | password | $@ flows to a logging call. | main.go:30:10:30:17 | password | Sensitive data returned by an access to password |
+| main.go:31:15:31:22 | password | main.go:31:15:31:22 | password | main.go:31:15:31:22 | password | $@ flows to a logging call. | main.go:31:15:31:22 | password | Sensitive data returned by an access to password |
+| main.go:32:11:32:18 | password | main.go:32:11:32:18 | password | main.go:32:11:32:18 | password | $@ flows to a logging call. | main.go:32:11:32:18 | password | Sensitive data returned by an access to password |
+| main.go:33:12:33:19 | password | main.go:33:12:33:19 | password | main.go:33:12:33:19 | password | $@ flows to a logging call. | main.go:33:12:33:19 | password | Sensitive data returned by an access to password |
+| main.go:34:10:34:17 | password | main.go:34:10:34:17 | password | main.go:34:10:34:17 | password | $@ flows to a logging call. | main.go:34:10:34:17 | password | Sensitive data returned by an access to password |
+| main.go:35:15:35:22 | password | main.go:35:15:35:22 | password | main.go:35:15:35:22 | password | $@ flows to a logging call. | main.go:35:15:35:22 | password | Sensitive data returned by an access to password |
+| main.go:36:11:36:18 | password | main.go:36:11:36:18 | password | main.go:36:11:36:18 | password | $@ flows to a logging call. | main.go:36:11:36:18 | password | Sensitive data returned by an access to password |
+| main.go:37:12:37:19 | password | main.go:37:12:37:19 | password | main.go:37:12:37:19 | password | $@ flows to a logging call. | main.go:37:12:37:19 | password | Sensitive data returned by an access to password |
+| main.go:38:10:38:17 | password | main.go:38:10:38:17 | password | main.go:38:10:38:17 | password | $@ flows to a logging call. | main.go:38:10:38:17 | password | Sensitive data returned by an access to password |
+| main.go:39:15:39:22 | password | main.go:39:15:39:22 | password | main.go:39:15:39:22 | password | $@ flows to a logging call. | main.go:39:15:39:22 | password | Sensitive data returned by an access to password |
+| main.go:40:11:40:18 | password | main.go:40:11:40:18 | password | main.go:40:11:40:18 | password | $@ flows to a logging call. | main.go:40:11:40:18 | password | Sensitive data returned by an access to password |
+| main.go:41:12:41:19 | password | main.go:41:12:41:19 | password | main.go:41:12:41:19 | password | $@ flows to a logging call. | main.go:41:12:41:19 | password | Sensitive data returned by an access to password |
+| main.go:42:14:42:21 | password | main.go:42:14:42:21 | password | main.go:42:14:42:21 | password | $@ flows to a logging call. | main.go:42:14:42:21 | password | Sensitive data returned by an access to password |
+| main.go:44:12:44:19 | password | main.go:44:12:44:19 | password | main.go:44:12:44:19 | password | $@ flows to a logging call. | main.go:44:12:44:19 | password | Sensitive data returned by an access to password |
+| main.go:45:17:45:24 | password | main.go:45:17:45:24 | password | main.go:45:17:45:24 | password | $@ flows to a logging call. | main.go:45:17:45:24 | password | Sensitive data returned by an access to password |
+| main.go:52:35:52:42 | password | main.go:52:35:52:42 | password | main.go:52:35:52:42 | password | $@ flows to a logging call. | main.go:52:35:52:42 | password | Sensitive data returned by an access to password |
 | overrides.go:13:14:13:23 | call to String | overrides.go:9:9:9:16 | password | overrides.go:13:14:13:23 | call to String | $@ flows to a logging call. | overrides.go:9:9:9:16 | password | Sensitive data returned by an access to password |
 | passwords.go:9:14:9:14 | x | passwords.go:30:8:30:15 | password | passwords.go:9:14:9:14 | x | $@ flows to a logging call. | passwords.go:30:8:30:15 | password | Sensitive data returned by an access to password |
 | passwords.go:25:14:25:21 | password | passwords.go:25:14:25:21 | password | passwords.go:25:14:25:21 | password | $@ flows to a logging call. | passwords.go:25:14:25:21 | password | Sensitive data returned by an access to password |
 | passwords.go:26:14:26:23 | selection of password | passwords.go:26:14:26:23 | selection of password | passwords.go:26:14:26:23 | selection of password | $@ flows to a logging call. | passwords.go:26:14:26:23 | selection of password | Sensitive data returned by an access to password |
 | passwords.go:27:14:27:26 | call to getPassword | passwords.go:27:14:27:26 | call to getPassword | passwords.go:27:14:27:26 | call to getPassword | $@ flows to a logging call. | passwords.go:27:14:27:26 | call to getPassword | Sensitive data returned by a call to getPassword |
 | passwords.go:28:14:28:28 | call to getPassword | passwords.go:28:14:28:28 | call to getPassword | passwords.go:28:14:28:28 | call to getPassword | $@ flows to a logging call. | passwords.go:28:14:28:28 | call to getPassword | Sensitive data returned by a call to getPassword |
-| passwords.go:28:14:28:28 | call to getPassword | util.go:16:9:16:18 | selection of password | passwords.go:28:14:28:28 | call to getPassword | $@ flows to a logging call. | util.go:16:9:16:18 | selection of password | Sensitive data returned by an access to password |
 | passwords.go:32:12:32:19 | password | passwords.go:32:12:32:19 | password | passwords.go:32:12:32:19 | password | $@ flows to a logging call. | passwords.go:32:12:32:19 | password | Sensitive data returned by an access to password |
 | passwords.go:34:14:34:35 | ...+... | passwords.go:34:28:34:35 | password | passwords.go:34:14:34:35 | ...+... | $@ flows to a logging call. | passwords.go:34:28:34:35 | password | Sensitive data returned by an access to password |
 | passwords.go:39:14:39:17 | obj1 | passwords.go:37:13:37:13 | x | passwords.go:39:14:39:17 | obj1 | $@ flows to a logging call. | passwords.go:37:13:37:13 | x | Sensitive data returned by an access to password |

diff --git a/go/ql/test/query-tests/Security/CWE-312/main.go b/go/ql/test/query-tests/Security/CWE-312/main.go
@@ -12,7 +12,34 @@ import (
 func main() {
 	password := "P4ssw0rd"
 
+	log.Print(password)
+	log.Printf("", password)
+	log.Printf(password, "")
 	log.Println(password)
+	log.Fatal(password)
+	log.Fatalf("", password)
+	log.Fatalf(password, "")
+	log.Fatalln(password)
+	log.Panic(password)
+	log.Panicf("", password)
+	log.Panicf(password, "")
+	log.Panicln(password)
+	log.Output(0, password)
+
+	l := log.Default()
+	l.Print(password)
+	l.Printf("", password)
+	l.Printf(password, "")
+	l.Println(password)
+	l.Fatal(password)
+	l.Fatalf("", password)
+	l.Fatalf(password, "")
+	l.Fatalln(password)
+	l.Panic(password)
+	l.Panicf("", password)
+	l.Panicf(password, "")
+	l.Panicln(password)
+	l.Output(0, password)
 
 	glog.Info(password)
 	logrus.Warning(password)