Rust: Query for cleartext logging of sensitive information

rust/ql/integration-tests/hello-project/summary.expected

@@ -14,11 +14,11 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| 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 | 1 |
 | Taint sources - active | 0 |
 | Taint sources - disabled | 0 |
 | Taint sources - sensitive data | 0 |

rust/ql/integration-tests/hello-workspace/summary.cargo.expected

@@ -14,11 +14,11 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| 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 | 1 |
 | Taint sources - active | 0 |
 | Taint sources - disabled | 0 |
 | Taint sources - sensitive data | 0 |

rust/ql/integration-tests/hello-workspace/summary.rust-project.expected

@@ -14,11 +14,11 @@
 | Macro calls - resolved | 2 |
 | Macro calls - total | 2 |
 | Macro calls - unresolved | 0 |
-| 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 | 1 |
 | Taint sources - active | 0 |
 | Taint sources - disabled | 0 |
 | Taint sources - sensitive data | 0 |

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/lib/codeql/rust/security/CleartextLoggingExtensions.qll

@@ -0,0 +1,40 @@
+/**
+ * Provides classes and predicates for reasoning about cleartext logging
+ * of sensitive information vulnerabilities.
+ */
+
+import rust
+private import codeql.rust.dataflow.DataFlow
+private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.security.SensitiveData
+
+/**
+ * Provides default sources, sinks and barriers for detecting cleartext logging
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module CleartextLogging {
+  /**
+   * A data flow source for cleartext logging vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for cleartext logging vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A barrier for cleartext logging vulnerabilities.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * Sensitive data, considered as a flow source.
+   */
+  private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
+
+  /** A sink for logging from model data. */
+  private class ModelsAsDataSinks extends Sink {
+    ModelsAsDataSinks() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
+  }
+}

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>
+</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/library-tests/dataflow/local/DataFlowStep.expected

@@ -1,11 +1,11 @@
 localStep
-| file://:0:0:0:0 | [summary param] 0 in lang:core::_::<crate::option::Option>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap_or | MaD:2 |
-| file://:0:0:0:0 | [summary param] 0 in lang:core::_::<crate::result::Result>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap_or | MaD:5 |
-| file://:0:0:0:0 | [summary param] 0 in lang:core::_::crate::hint::must_use | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::crate::hint::must_use | MaD:7 |
-| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap | MaD:1 |
-| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap_or | MaD:3 |
-| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::result::Result::Ok(0)] in lang:core::_::<crate::result::Result>::unwrap | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap | MaD:4 |
-| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::result::Result::Ok(0)] in lang:core::_::<crate::result::Result>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap_or | MaD:6 |
+| file://:0:0:0:0 | [summary param] 0 in lang:core::_::<crate::option::Option>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap_or | MaD:3 |
+| file://:0:0:0:0 | [summary param] 0 in lang:core::_::<crate::result::Result>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap_or | MaD:6 |
+| file://:0:0:0:0 | [summary param] 0 in lang:core::_::crate::hint::must_use | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::crate::hint::must_use | MaD:8 |
+| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap | MaD:2 |
+| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::option::Option>::unwrap_or | MaD:4 |
+| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::result::Result::Ok(0)] in lang:core::_::<crate::result::Result>::unwrap | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap | MaD:5 |
+| file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::result::Result::Ok(0)] in lang:core::_::<crate::result::Result>::unwrap_or | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:core::_::<crate::result::Result>::unwrap_or | MaD:7 |
 | main.rs:3:11:3:11 | [SSA] i | main.rs:4:12:4:12 | i |  |
 | main.rs:3:11:3:11 | i | main.rs:3:11:3:11 | [SSA] i |  |
 | main.rs:3:11:3:16 | ...: i64 | main.rs:3:11:3:11 | i |  |
@@ -14,6 +14,7 @@ localStep
 | main.rs:7:9:7:9 | s | main.rs:7:9:7:9 | [SSA] s |  |
 | main.rs:7:9:7:14 | ...: i64 | main.rs:7:9:7:9 | s |  |
 | main.rs:8:14:8:20 | FormatArgsExpr | main.rs:8:14:8:20 | MacroExpr |  |
+| main.rs:8:14:8:20 | MacroExpr | main.rs:8:5:8:21 | ...::_print | MaD:1 |
 | main.rs:19:9:19:9 | [SSA] s | main.rs:20:10:20:10 | s |  |
 | main.rs:19:9:19:9 | s | main.rs:19:9:19:9 | [SSA] s |  |
 | main.rs:19:13:19:21 | source(...) | main.rs:19:9:19:9 | s |  |
@@ -468,13 +469,14 @@ localStep
 | main.rs:436:13:436:33 | result_questionmark(...) | main.rs:436:9:436:9 | _ |  |
 | main.rs:448:36:448:41 | ...::new(...) | main.rs:448:36:448:41 | MacroExpr |  |
 models
-| 1 | Summary: lang:core; <crate::option::Option>::unwrap; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
-| 2 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[0]; ReturnValue; value |
-| 3 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
-| 4 | Summary: lang:core; <crate::result::Result>::unwrap; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
-| 5 | Summary: lang:core; <crate::result::Result>::unwrap_or; Argument[0]; ReturnValue; value |
-| 6 | Summary: lang:core; <crate::result::Result>::unwrap_or; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
-| 7 | Summary: lang:core; crate::hint::must_use; Argument[0]; ReturnValue; value |
+| 1 | Sink: lang:std; crate::io::stdio::_print; log-injection; Argument[0] |
+| 2 | Summary: lang:core; <crate::option::Option>::unwrap; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
+| 3 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[0]; ReturnValue; value |
+| 4 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
+| 5 | Summary: lang:core; <crate::result::Result>::unwrap; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
+| 6 | Summary: lang:core; <crate::result::Result>::unwrap_or; Argument[0]; ReturnValue; value |
+| 7 | Summary: lang:core; <crate::result::Result>::unwrap_or; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
+| 8 | Summary: lang:core; crate::hint::must_use; Argument[0]; ReturnValue; value |
 storeStep
 | file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text |
 | main.rs:94:14:94:22 | source(...) | tuple.0 | main.rs:94:13:94:26 | TupleExpr |

rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected

@@ -1,7 +1,8 @@
 additionalTaintStep
-| file://:0:0:0:0 | [summary param] 0 in lang:alloc::_::crate::fmt::format | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::crate::fmt::format | MaD:2 |
-| file://:0:0:0:0 | [summary param] self in lang:alloc::_::<crate::string::String>::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::<crate::string::String>::as_str | MaD:1 |
-| file://:0:0:0:0 | [summary param] self in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | MaD:3 |
+| file://:0:0:0:0 | [summary param] 0 in lang:alloc::_::crate::fmt::format | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::crate::fmt::format | MaD:3 |
+| file://:0:0:0:0 | [summary param] self in lang:alloc::_::<crate::string::String>::as_bytes | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::<crate::string::String>::as_bytes | MaD:1 |
+| file://:0:0:0:0 | [summary param] self in lang:alloc::_::<crate::string::String>::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::<crate::string::String>::as_str | MaD:2 |
+| file://:0:0:0:0 | [summary param] self in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | MaD:4 |
 | main.rs:4:5:4:8 | 1000 | main.rs:4:5:4:12 | ... + ... |  |
 | main.rs:4:12:4:12 | i | main.rs:4:5:4:12 | ... + ... |  |
 | main.rs:8:20:8:20 | s | main.rs:8:14:8:20 | FormatArgsExpr |  |
@@ -19,6 +20,7 @@ additionalTaintStep
 | main.rs:64:24:64:27 | s[1] | main.rs:64:18:64:27 | FormatArgsExpr |  |
 | main.rs:69:9:69:12 | arr2 | main.rs:69:9:69:15 | arr2[1] |  |
 models
-| 1 | Summary: lang:alloc; <crate::string::String>::as_str; Argument[self]; ReturnValue; taint |
-| 2 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint |
-| 3 | Summary: repo:https://github.com/seanmonstar/reqwest:reqwest; <crate::blocking::response::Response>::text; Argument[self]; ReturnValue.Variant[crate::result::Result::Ok(0)]; taint |
+| 1 | Summary: lang:alloc; <crate::string::String>::as_bytes; Argument[self]; ReturnValue; taint |
+| 2 | Summary: lang:alloc; <crate::string::String>::as_str; Argument[self]; ReturnValue; taint |
+| 3 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint |
+| 4 | Summary: repo:https://github.com/seanmonstar/reqwest:reqwest; <crate::blocking::response::Response>::text; Argument[self]; ReturnValue.Variant[crate::result::Result::Ok(0)]; taint |

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 |

rust/ql/test/query-tests/security/CWE-312/CONSISTENCY/ExtractionConsistency.expected

@@ -0,0 +1,3 @@
+extractionWarning
+| test_logging.rs:90:12:90:30 | expected R_PAREN |
+| test_logging.rs:90:12:90:30 | macro expansion failed: the macro '$crate::__private_api::format_args' expands to ERROR but a Expr was expected |