Rust: Query for cleartext logging of sensitive information

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -773,7 +773,7 @@ final class ArrayElementContent extends Content, TArrayElement {
  * NOTE: Unlike `struct`s and `enum`s tuples are structural and not nominal,
  * hence we don't store a canonical path for them.
  */
-private class TuplePositionContent extends Content, TTuplePositionContent {
+final class TuplePositionContent extends Content, TTuplePositionContent {
   private int pos;
 
   TuplePositionContent() { this = TTuplePositionContent(pos) }

rust/ql/lib/codeql/rust/frameworks/log.model.yml

@@ -0,0 +1,17 @@
+extensions:
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: sinkModel
+    data:
+      - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[0]", "log-injection", "manual"] # args
+      - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[2]", "log-injection", "manual"] # target
+      - ["repo:https://github.com/rust-lang/log:log", "crate::__private_api::log", "Argument[3]", "log-injection", "manual"] # key value
+      - ["lang:std", "crate::io::stdio::_print", "Argument[0]", "log-injection", "manual"]
+      - ["lang:std", "crate::io::stdio::_eprint", "Argument[0]", "log-injection", "manual"]
+      - ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"]
+      - ["lang:std", "<crate::io::stdio::StdoutLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"]
+      - ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write", "Argument[0]", "log-injection", "manual"]
+      - ["lang:std", "<crate::io::stdio::StderrLock as crate::io::Write>::write_all", "Argument[0]", "log-injection", "manual"]
+      - ["lang:core", "crate::panicking::panic_fmt", "Argument[0]", "log-injection", "manual"]
+      - ["lang:core", "crate::panicking::assert_failed", "Argument[3].Variant[crate::option::Option::Some(0)]", "log-injection", "manual"]
+      - ["lang:core", "<crate::option::Option>::expect", "Argument[0]", "log-injection", "manual"]

rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml

@@ -13,6 +13,7 @@ extensions:
       - ["lang:core", "<crate::result::Result>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
       # String
       - ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]
+      - ["lang:alloc", "<crate::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "manual"]
       # Hint
       - ["lang:core", "crate::hint::must_use", "Argument[0]", "ReturnValue", "value", "manual"]
       # Fmt

rust/ql/src/queries/security/CWE-312/CleartextLogging.qhelp

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Sensitive user data and system information that is logged could be seen by an attacker when it is
+displayed. Also, external processes often store the standard output and standard error streams of
+an application, which will include logged sensitive information.</p>
+</p>
+</overview>
+
+<recommendation>
+<p>
+Do not log sensitive data. If it is necessary to log sensitive data, encrypt it before logging.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example code logs user credentials (in this case, their password) in plaintext:
+</p>
+<sample src="CleartextLoggingBad.rs"/>
+<p>
+Instead, you should encrypt the credentials, or better still omit them entirely:
+</p>
+<sample src="CleartextLoggingGood.rs"/>
+</example>
+
+<references>
+
+<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li>
+<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
+<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html#data-to-exclude">Logging Cheat Sheet - Data to exclude</a>.<li>
+
+</references>
+</qhelp>

rust/ql/src/queries/security/CWE-312/CleartextLogging.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Cleartext logging of sensitive information
+ * @description Logging sensitive information in plaintext can
+ *              expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id rust/cleartext-logging
+ * @tags security
+ *       external/cwe/cwe-312
+ *       external/cwe/cwe-359
+ *       external/cwe/cwe-532
+ */
+
+import rust
+import codeql.rust.security.CleartextLoggingExtensions
+import codeql.rust.dataflow.DataFlow
+import codeql.rust.dataflow.TaintTracking
+import codeql.rust.dataflow.internal.DataFlowImpl
+
+/**
+ * A taint-tracking configuration for cleartext logging vulnerabilities.
+ */
+module CleartextLoggingConfig implements DataFlow::ConfigSig {
+  import CleartextLogging
+
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
+
+  predicate isBarrierIn(DataFlow::Node node) {
+    // make sources barriers so that we only report the closest instance
+    isSource(node)
+  }
+
+  predicate isAdditionalFlowStep(Node node1, Node node2) {
+    // flow from `a` to `&a`
+    node2.asExpr().getExpr().(RefExpr).getExpr() = node1.asExpr().getExpr()
+  }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
+    // flow out from tuple content at sinks.
+    isSink(node) and
+    c.getAReadContent() instanceof TuplePositionContent
+  }
+}
+
+module CleartextLoggingFlow = TaintTracking::Global<CleartextLoggingConfig>;
+
+import CleartextLoggingFlow::PathGraph
+
+from CleartextLoggingFlow::PathNode source, CleartextLoggingFlow::PathNode sink
+where CleartextLoggingFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "This operation writes $@ to a log file.", source,
+  source.getNode().toString()

rust/ql/src/queries/security/CWE-312/CleartextLoggingBad.rs

@@ -0,0 +1,2 @@
+let password = "P@ssw0rd"
+info!("User password changed to {password}")

rust/ql/src/queries/security/CWE-312/CleartextLoggingGood.rs

@@ -0,0 +1,2 @@
+let password = "P@ssw0rd"
+info!("User password changed")

rust/ql/src/queries/summary/Stats.qll

@@ -10,6 +10,7 @@ private import codeql.rust.AstConsistency as AstConsistency
 private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency
 private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency
 private import codeql.rust.security.SqlInjectionExtensions
+private import codeql.rust.security.CleartextLoggingExtensions
 
 /**
  * Gets a count of the total number of lines of code in the database.
@@ -58,7 +59,9 @@ int getTaintEdgesCount() {
  * Gets a kind of query for which `n` is a sink (if any).
  */
 string getAQuerySinkKind(DataFlow::Node n) {
-  (n instanceof SqlInjection::Sink and result = "SqlInjection")
+  n instanceof SqlInjection::Sink and result = "SqlInjection"
+  or
+  n instanceof CleartextLogging::Sink and result = "CleartextLogging"
 }
 
 /**

rust/ql/test/query-tests/diagnostics/SummaryStats.expected

@@ -14,11 +14,11 @@
 | Macro calls - resolved | 8 |
 | Macro calls - total | 9 |
 | Macro calls - unresolved | 1 |
-| Taint edges - number of edges | 2 |
+| Taint edges - number of edges | 3 |
 | Taint reach - nodes tainted | 0 |
 | Taint reach - per million nodes | 0 |
 | Taint sinks - cryptographic operations | 0 |
-| Taint sinks - query sinks | 0 |
+| Taint sinks - query sinks | 3 |
 | Taint sources - active | 0 |
 | Taint sources - disabled | 0 |
 | Taint sources - sensitive data | 0 |