Merge pull request #20829 from geoffw0/cert-checks

geoffw0 · web-flow · commit daead038abda · 2025-11-24T15:21:58.000Z
Rust: New Query rust/disabled-certificate-check
diff --git a/rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected b/rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected
@@ -11,6 +11,7 @@ ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
 ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
 ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
 ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
+ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
 ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
diff --git a/rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected b/rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected
@@ -12,6 +12,7 @@ ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
 ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
 ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
 ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
+ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
 ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
diff --git a/rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected b/rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected
@@ -12,6 +12,7 @@ ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
 ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
 ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
 ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
+ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
 ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
 ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
diff --git a/rust/ql/lib/change-notes/2025-11-21-fs.md b/rust/ql/lib/change-notes/2025-11-21-fs.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added more detailed models for `std::fs` and `std::path`.
diff --git a/rust/ql/lib/codeql/rust/frameworks/native-tls.model.yml b/rust/ql/lib/codeql/rust/frameworks/native-tls.model.yml
@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/rust-all
+      extensible: sinkModel
+    data:
+      - ["<native_tls::TlsConnectorBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
+      - ["<native_tls::TlsConnectorBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
+      - ["<async_native_tls::connect::TlsConnector>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
+      - ["<async_native_tls::connect::TlsConnector>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
diff --git a/rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml b/rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml
@@ -11,6 +11,10 @@ extensions:
     data:
       - ["<reqwest::async_impl::client::Client>::request", "Argument[1]", "request-url", "manual"]
       - ["<reqwest::blocking::client::Client>::request", "Argument[1]", "request-url", "manual"]
+      - ["<reqwest::async_impl::client::ClientBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
+      - ["<reqwest::async_impl::client::ClientBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
+      - ["<reqwest::blocking::client::ClientBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
+      - ["<reqwest::blocking::client::ClientBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
   - addsTo:
       pack: codeql/rust-all
       extensible: summaryModel
diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml b/rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml
@@ -3,14 +3,27 @@ extensions:
       pack: codeql/rust-all
       extensible: sourceModel
     data:
+      - ["std::fs::exists", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["std::fs::read", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["std::fs::read_dir", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["std::fs::read_to_string", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["std::fs::read_link", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["std::fs::metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["std::fs::symlink_metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["<std::fs::DirEntry>::path", "ReturnValue", "file", "manual"]
       - ["<std::fs::DirEntry>::file_name", "ReturnValue", "file", "manual"]
       - ["<std::fs::File>::open", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["<std::fs::File>::open_buffered", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
       - ["<std::fs::OpenOptions>::open", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["<std::path::Path>::exists", "ReturnValue", "file", "manual"]
+      - ["<std::path::Path>::try_exists", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["<std::path::Path>::is_file", "ReturnValue", "file", "manual"]
+      - ["<std::path::Path>::is_dir", "ReturnValue", "file", "manual"]
+      - ["<std::path::Path>::is_symlink", "ReturnValue", "file", "manual"]
+      - ["<std::path::Path>::metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["<std::path::Path>::symlink_metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["<std::path::Path>::read_dir", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
+      - ["<std::path::Path>::read_link", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
   - addsTo:
       pack: codeql/rust-all
       extensible: sinkModel
@@ -68,3 +81,12 @@ extensions:
       - ["<std::path::Path>::with_extension", "Argument[Self].Reference", "ReturnValue", "taint", "manual"]
       - ["<std::path::Path>::with_file_name", "Argument[Self].Reference", "ReturnValue", "taint", "manual"]
       - ["<std::path::Path>::with_file_name", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::accessed", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
+      - ["<std::fs::Metadata>::created", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
+      - ["<std::fs::Metadata>::file_type", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::is_file", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::is_dir", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::is_symlink", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::len", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
+      - ["<std::fs::Metadata>::modified", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
+      - ["<std::fs::Metadata>::permissions", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
diff --git a/rust/ql/lib/codeql/rust/security/DisabledCertificateCheckExtensions.qll b/rust/ql/lib/codeql/rust/security/DisabledCertificateCheckExtensions.qll
@@ -0,0 +1,45 @@
+/**
+ * Provides classes and predicates for reasoning about disabled certificate
+ * check vulnerabilities.
+ */
+
+import rust
+private import codeql.rust.dataflow.DataFlow
+private import codeql.rust.dataflow.FlowSink
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.internal.Node as Node
+
+/**
+ * Provides default sinks for detecting disabled certificate check
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module DisabledCertificateCheckExtensions {
+  /**
+   * A data flow sink for disabled certificate check vulnerabilities.
+   */
+  abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "DisabledCertificateCheck" }
+  }
+
+  /**
+   * A sink for disabled certificate check vulnerabilities from model data.
+   */
+  private class ModelsAsDataSink extends Sink {
+    ModelsAsDataSink() { sinkNode(this, "disable-certificate") }
+  }
+
+  /**
+   * A heuristic sink for disabled certificate check vulnerabilities based on function names.
+   */
+  private class HeuristicSink extends Sink {
+    HeuristicSink() {
+      exists(CallExprBase fc |
+        fc.getStaticTarget().(Function).getName().getText() =
+          ["danger_accept_invalid_certs", "danger_accept_invalid_hostnames"] and
+        fc.getArg(0) = this.asExpr() and
+        // don't duplicate modeled sinks
+        not exists(ModelsAsDataSink s | s.(Node::FlowSummaryNode).getSinkElement().getCall() = fc)
+      )
+    }
+  }
+}
diff --git a/rust/ql/src/change-notes/2025-11-12-disabled-certificate-check.md b/rust/ql/src/change-notes/2025-11-12-disabled-certificate-check.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query `rust/disabled-certificate-check`, to detect disabled TLS certificate checks.
diff --git a/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.qhelp b/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.qhelp
@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+The <code>danger_accept_invalid_certs</code> option on TLS connectors and HTTP clients controls whether certificate verification is performed. If this option is set to <code>true</code>, the client will accept any certificate, making it susceptible to man-in-the-middle attacks.
+</p>
+<p>
+Similarly, the <code>danger_accept_invalid_hostnames</code> option controls whether hostname verification is performed. If this option is set to <code>true</code>, the client will accept any valid certificate regardless of the site that certificate is for, again making it susceptible to man-in-the-middle attacks.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Do not set <code>danger_accept_invalid_certs</code> or <code>danger_accept_invalid_hostnames</code> to <code>true</code>, except in controlled environments such as tests. In production, always ensure certificate and hostname verification is enabled to prevent security risks.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following code snippet shows a function that creates an HTTP client with certificate verification disabled:
+</p>
+<sample src="DisabledCertificateCheckBad.rs"/>
+<p>
+In production code, always configure clients to verify certificates:
+</p>
+<sample src="DisabledCertificateCheckGood.rs"/>
+</example>
+<references>
+<li>
+Rust native-tls crate: <a href="https://docs.rs/native-tls/latest/native_tls/struct.TlsConnectorBuilder.html">TlsConnectorBuilder</a>.
+</li>
+<li>
+Rust reqwest crate: <a href="https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html">ClientBuilder</a>.
+</li>
+<li>
+SSL.com: <a href="https://www.ssl.com/article/browsers-and-certificate-validation/">Browsers and Certificate Validation</a>.
+</li>
+</references>
+</qhelp>
diff --git a/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql b/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
@@ -0,0 +1,47 @@
+/**
+ * @name Disabled TLS certificate check
+ * @description An application that disables TLS certificate checking is more vulnerable to
+ *              man-in-the-middle attacks.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id rust/disabled-certificate-check
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+
+import rust
+import codeql.rust.dataflow.DataFlow
+import codeql.rust.dataflow.TaintTracking
+import codeql.rust.security.DisabledCertificateCheckExtensions
+import codeql.rust.Concepts
+
+/**
+ * A taint configuration for disabled TLS certificate checks.
+ */
+module DisabledCertificateCheckConfig implements DataFlow::ConfigSig {
+  import DisabledCertificateCheckExtensions
+
+  predicate isSource(DataFlow::Node node) {
+    // the constant `true`
+    node.asExpr().(BooleanLiteralExpr).getTextValue() = "true"
+    or
+    // a value controlled by a potential attacker
+    node instanceof ActiveThreatModelSource
+  }
+
+  predicate isSink(DataFlow::Node node) { node instanceof Sink }
+
+  predicate observeDiffInformedIncrementalMode() { any() }
+}
+
+module DisabledCertificateCheckFlow = TaintTracking::Global<DisabledCertificateCheckConfig>;
+
+import DisabledCertificateCheckFlow::PathGraph
+
+from
+  DisabledCertificateCheckFlow::PathNode sourceNode, DisabledCertificateCheckFlow::PathNode sinkNode
+where DisabledCertificateCheckFlow::flowPath(sourceNode, sinkNode)
+select sinkNode.getNode(), sourceNode, sinkNode,
+  "Disabling TLS certificate validation can expose the application to man-in-the-middle attacks."
diff --git a/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckBad.rs b/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckBad.rs
@@ -0,0 +1,6 @@
+// BAD: Disabling certificate validation in Rust
+
+let _client = reqwest::Client::builder()
+    .danger_accept_invalid_certs(true) // disables certificate validation
+    .build()
+    .unwrap();
diff --git a/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckGood.rs b/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckGood.rs
@@ -0,0 +1,10 @@
+// GOOD: Certificate validation is enabled (default)
+
+let _client = reqwest::Client::builder()
+    .danger_accept_invalid_certs(false) // certificate validation enabled explicitly
+    .build()
+    .unwrap();
+
+let _client = native_tls::TlsConnector::builder() // certificate validation enabled by default
+    .build()
+    .unwrap();
diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll
@@ -22,6 +22,7 @@ private import codeql.rust.security.AccessInvalidPointerExtensions
 private import codeql.rust.security.CleartextLoggingExtensions
 private import codeql.rust.security.CleartextStorageDatabaseExtensions
 private import codeql.rust.security.CleartextTransmissionExtensions
+private import codeql.rust.security.DisabledCertificateCheckExtensions
 private import codeql.rust.security.HardcodedCryptographicValueExtensions
 private import codeql.rust.security.InsecureCookieExtensions
 private import codeql.rust.security.LogInjectionExtensions
diff --git a/rust/ql/test/library-tests/dataflow/sources/file/TaintSources.expected b/rust/ql/test/library-tests/dataflow/sources/file/TaintSources.expected
@@ -4,8 +4,10 @@
 | test.rs:17:31:17:38 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:22:22:22:39 | ...::read_to_string | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:22:22:22:39 | ...::read_to_string | Flow source 'FileSource' of type file (DEFAULT). |
+| test.rs:26:18:26:29 | ...::read_dir | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:29:22:29:25 | path | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:43:27:43:35 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
+| test.rs:51:52:51:59 | read_dir | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:54:22:54:25 | path | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:55:27:55:35 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:65:22:65:34 | ...::read_link | Flow source 'FileSource' of type file (DEFAULT). |
diff --git a/rust/ql/test/library-tests/dataflow/sources/file/test.rs b/rust/ql/test/library-tests/dataflow/sources/file/test.rs
@@ -23,7 +23,7 @@ fn test_fs() -> Result<(), Box<dyn std::error::Error>> {
         sink(buffer); // $ hasTaintFlow="file.txt"
     }
 
-    for entry in fs::read_dir("directory")? {
+    for entry in fs::read_dir("directory")? { // $ Alert[rust/summary/taint-sources]
         let e = entry?;
 
         let path = e.path(); // $ Alert[rust/summary/taint-sources]
@@ -48,7 +48,7 @@ fn test_fs() -> Result<(), Box<dyn std::error::Error>> {
         sink(file_name.clone().as_encoded_bytes()); // $ MISSING: hasTaintFlow
         sink(file_name); // $ hasTaintFlow
     }
-    for entry in std::path::Path::new("directory").read_dir()? {
+    for entry in std::path::Path::new("directory").read_dir()? { // $ Alert[rust/summary/taint-sources]
         let e = entry?;
 
         let path = e.path(); // $ Alert[rust/summary/taint-sources]
diff --git a/rust/ql/test/query-tests/security/CWE-295/Cargo.lock b/rust/ql/test/query-tests/security/CWE-295/Cargo.lock
diff --git a/rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.expected b/rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.expected
diff --git a/rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.qlref b/rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.qlref
diff --git a/rust/ql/test/query-tests/security/CWE-295/main.rs b/rust/ql/test/query-tests/security/CWE-295/main.rs
diff --git a/rust/ql/test/query-tests/security/CWE-295/options.yml b/rust/ql/test/query-tests/security/CWE-295/options.yml

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* Added more detailed models for `std::fs` and `std::path`.