Skip to content

Commit daead03

Browse files
geoffw0geoffw0
authored
Merge pull request #20829 from geoffw0/cert-checks
Rust: New Query rust/disabled-certificate-check
2 parents 555301c + b62968f commit daead03

21 files changed

+2092
-2
lines changed

‎rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
14+
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
1415
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1516
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1617
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql

‎rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1414
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
15+
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
1516
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1617
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1718
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql

‎rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1414
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
15+
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
1516
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1617
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
1718
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql

‎rust/ql/lib/change-notes/2025-11-21-fs.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added more detailed models for `std::fs` and `std::path`.

‎rust/ql/lib/codeql/rust/frameworks/native-tls.model.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/rust-all
4+
extensible: sinkModel
5+
data:
6+
- ["<native_tls::TlsConnectorBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
7+
- ["<native_tls::TlsConnectorBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
8+
- ["<async_native_tls::connect::TlsConnector>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
9+
- ["<async_native_tls::connect::TlsConnector>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]

‎rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ extensions:
1111
data:
1212
- ["<reqwest::async_impl::client::Client>::request", "Argument[1]", "request-url", "manual"]
1313
- ["<reqwest::blocking::client::Client>::request", "Argument[1]", "request-url", "manual"]
14+
- ["<reqwest::async_impl::client::ClientBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
15+
- ["<reqwest::async_impl::client::ClientBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
16+
- ["<reqwest::blocking::client::ClientBuilder>::danger_accept_invalid_certs", "Argument[0]", "disable-certificate", "manual"]
17+
- ["<reqwest::blocking::client::ClientBuilder>::danger_accept_invalid_hostnames", "Argument[0]", "disable-certificate", "manual"]
1418
- addsTo:
1519
pack: codeql/rust-all
1620
extensible: summaryModel

‎rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,27 @@ extensions:
33
pack: codeql/rust-all
44
extensible: sourceModel
55
data:
6+
- ["std::fs::exists", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
67
- ["std::fs::read", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
8+
- ["std::fs::read_dir", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
79
- ["std::fs::read_to_string", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
810
- ["std::fs::read_link", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
11+
- ["std::fs::metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
12+
- ["std::fs::symlink_metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
913
- ["<std::fs::DirEntry>::path", "ReturnValue", "file", "manual"]
1014
- ["<std::fs::DirEntry>::file_name", "ReturnValue", "file", "manual"]
1115
- ["<std::fs::File>::open", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
1216
- ["<std::fs::File>::open_buffered", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
1317
- ["<std::fs::OpenOptions>::open", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
18+
- ["<std::path::Path>::exists", "ReturnValue", "file", "manual"]
19+
- ["<std::path::Path>::try_exists", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
20+
- ["<std::path::Path>::is_file", "ReturnValue", "file", "manual"]
21+
- ["<std::path::Path>::is_dir", "ReturnValue", "file", "manual"]
22+
- ["<std::path::Path>::is_symlink", "ReturnValue", "file", "manual"]
23+
- ["<std::path::Path>::metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
24+
- ["<std::path::Path>::symlink_metadata", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
25+
- ["<std::path::Path>::read_dir", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
26+
- ["<std::path::Path>::read_link", "ReturnValue.Field[core::result::Result::Ok(0)]", "file", "manual"]
1427
- addsTo:
1528
pack: codeql/rust-all
1629
extensible: sinkModel
@@ -68,3 +81,12 @@ extensions:
6881
- ["<std::path::Path>::with_extension", "Argument[Self].Reference", "ReturnValue", "taint", "manual"]
6982
- ["<std::path::Path>::with_file_name", "Argument[Self].Reference", "ReturnValue", "taint", "manual"]
7083
- ["<std::path::Path>::with_file_name", "Argument[0]", "ReturnValue", "taint", "manual"]
84+
- ["<std::fs::Metadata>::accessed", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
85+
- ["<std::fs::Metadata>::created", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
86+
- ["<std::fs::Metadata>::file_type", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
87+
- ["<std::fs::Metadata>::is_file", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
88+
- ["<std::fs::Metadata>::is_dir", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
89+
- ["<std::fs::Metadata>::is_symlink", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
90+
- ["<std::fs::Metadata>::len", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
91+
- ["<std::fs::Metadata>::modified", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
92+
- ["<std::fs::Metadata>::permissions", "Argument[self].Reference", "ReturnValue", "taint", "manual"]

‎rust/ql/lib/codeql/rust/security/DisabledCertificateCheckExtensions.qll‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* Provides classes and predicates for reasoning about disabled certificate
3+
* check vulnerabilities.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.FlowSink
9+
private import codeql.rust.Concepts
10+
private import codeql.rust.dataflow.internal.Node as Node
11+
12+
/**
13+
* Provides default sinks for detecting disabled certificate check
14+
* vulnerabilities, as well as extension points for adding your own.
15+
*/
16+
module DisabledCertificateCheckExtensions {
17+
/**
18+
* A data flow sink for disabled certificate check vulnerabilities.
19+
*/
20+
abstract class Sink extends QuerySink::Range {
21+
override string getSinkType() { result = "DisabledCertificateCheck" }
22+
}
23+
24+
/**
25+
* A sink for disabled certificate check vulnerabilities from model data.
26+
*/
27+
private class ModelsAsDataSink extends Sink {
28+
ModelsAsDataSink() { sinkNode(this, "disable-certificate") }
29+
}
30+
31+
/**
32+
* A heuristic sink for disabled certificate check vulnerabilities based on function names.
33+
*/
34+
private class HeuristicSink extends Sink {
35+
HeuristicSink() {
36+
exists(CallExprBase fc |
37+
fc.getStaticTarget().(Function).getName().getText() =
38+
["danger_accept_invalid_certs", "danger_accept_invalid_hostnames"] and
39+
fc.getArg(0) = this.asExpr() and
40+
// don't duplicate modeled sinks
41+
not exists(ModelsAsDataSink s | s.(Node::FlowSummaryNode).getSinkElement().getCall() = fc)
42+
)
43+
}
44+
}
45+
}

‎rust/ql/src/change-notes/2025-11-12-disabled-certificate-check.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query `rust/disabled-certificate-check`, to detect disabled TLS certificate checks.

‎rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.qhelp‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
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.
9+
</p>
10+
<p>
11+
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.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
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.
18+
</p>
19+
</recommendation>
20+
21+
<example>
22+
<p>
23+
The following code snippet shows a function that creates an HTTP client with certificate verification disabled:
24+
</p>
25+
<sample src="DisabledCertificateCheckBad.rs"/>
26+
<p>
27+
In production code, always configure clients to verify certificates:
28+
</p>
29+
<sample src="DisabledCertificateCheckGood.rs"/>
30+
</example>
31+
<references>
32+
<li>
33+
Rust native-tls crate: <a href="https://docs.rs/native-tls/latest/native_tls/struct.TlsConnectorBuilder.html">TlsConnectorBuilder</a>.
34+
</li>
35+
<li>
36+
Rust reqwest crate: <a href="https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html">ClientBuilder</a>.
37+
</li>
38+
<li>
39+
SSL.com: <a href="https://www.ssl.com/article/browsers-and-certificate-validation/">Browsers and Certificate Validation</a>.
40+
</li>
41+
</references>
42+
</qhelp>

0 commit comments

Comments
 (0)