Rust: New Query rust/disabled-certificate-check

[geoffw0]

Adds a new Rust query rust/disabled-certificate-check, to detect disabled TLS certificate checks. Based on go/disabled-certificate-check though there are some differences in both the sinks and query logic. Includes tests and .qhelp with examples.

On the MRVA-1000, we find 73 results across 29 projects. A sample of these results LGTM (though a good number are only in tests; only a minority needed data flow to be found).

[github-actions]

QHelp previews:

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

Disabled TLS certificate check

The danger_accept_invalid_certs option on TLS connectors and HTTP clients controls whether certificate verification is performed. If this option is set to true, the client will accept any certificate, making it susceptible to man-in-the-middle attacks.

Similarly, the danger_accept_invalid_hostnames option controls whether hostname verification is performed. If this option is set to true, 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.

Recommendation

Do not set danger_accept_invalid_certs or danger_accept_invalid_hostnames to true, except in controlled environments such as tests. In production, always ensure certificate and hostname verification is enabled to prevent security risks.

Example

The following code snippet shows a function that creates an HTTP client with certificate verification disabled:

// BAD: Disabling certificate validation in Rust

let _client = reqwest::Client::builder()
    .danger_accept_invalid_certs(true) // disables certificate validation
    .build()
    .unwrap();

In production code, always configure clients to verify certificates:

// 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();

References

• Rust native-tls crate: TlsConnectorBuilder.
• Rust reqwest crate: ClientBuilder.
• SSL.com: Browsers and Certificate Validation.
• Common Weakness Enumeration: CWE-295.

[geoffw0]

I did try defining the sinks using models-as-data, but it proved to be less reliable in the absence of full modelling of the classes involved. For example in a chain Builder.a().b().sink(true), we need summaries for a() and b() to understand that sink(true) is being called on a Builder. With the QL-defined sinks that only match the function names, these models are not required - and given the very specific names of these functions, matching with just names is likely to be very accurate.

[paldepind]

I'm not sure I follow this. Since the sink here uses getStaticTarget it only works when we can resolve a target. Wouldn't we then also be able to find the MaD in the same cases as MaD works by finding the target and checking its canonical name?

[geoffw0]

Hmm, I think you're right, maybe something slightly different is going on. I will investigate...

[geoffw0]

I guess I must've made a mistake in my initial MaD implementation. My second attempt (now pushed) works fine. I did find a couple of additional sinks with a matching name and purpose in the MRVA-1000, so I've modelled them as well.

[geoffw0]

... after experimenting with this some more, we still lose quite a lot of sinks with the modelled sinks compared to the old name matching (e.g. the MRVA-1000 alone would require 10+ more models above what I'd already added). And the name matching is quite precise due to those long and specific function names. So I've reinstated that, but I'm calling it a heuristic sink now. If you disable the heuristic sinks, we do still have reliable MaD sinks for the common cases.

[Copilot]

Pull Request Overview

This PR introduces a new security query rust/disabled-certificate-check to detect when TLS certificate validation is disabled in Rust code. The query identifies calls to danger_accept_invalid_certs and danger_accept_invalid_hostnames methods that accept true as an argument, which disables important security checks.

• Implements data flow analysis to track true literals flowing into certificate validation configuration methods
• Supports detection across popular Rust HTTP/TLS libraries (reqwest, native-tls)
• Includes comprehensive test cases with expected results

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rust/ql/test/query-tests/security/CWE-295/options.yml	Configures test dependencies for reqwest, native-tls, and rand crates
rust/ql/test/query-tests/security/CWE-295/main.rs	Test cases covering native-tls, reqwest, and data flow scenarios
rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.qlref	Query reference for running the test
rust/ql/test/query-tests/security/CWE-295/DisabledCertificateCheck.expected	Expected test results with 14 findings
rust/ql/test/query-tests/security/CWE-295/Cargo.lock	Generated lock file with resolved dependencies
rust/ql/src/queries/summary/Stats.qll	Adds import for new security extension module
rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckGood.rs	Example of safe certificate validation configuration
rust/ql/src/queries/security/CWE-295/DisabledCertificateCheckBad.rs	Example of unsafe certificate validation disabling
rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql	Main query implementation using data flow analysis
rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.qhelp	Help documentation explaining the vulnerability and remediation
rust/ql/src/change-notes/2025-11-12-disabled-certificate-check.md	Change note documenting the new query
rust/ql/lib/codeql/rust/security/DisabledCertificateCheckExtensions.qll	Extension library defining sinks for certificate check configuration

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mchammer01]

I'll review this on behalf of Docs today.

[mchammer01]

@geoffw0 - this LGTM. I made a few minor editorial suggestions. As usual, feel free to ignore anything you don't agree with.

Approving on behalf of Docs.

[geoffw0]

Thanks for the review and suggestions @mchammer01 !

[mchammer01]

Thanks for looking at my suggestions and for improving the qhelp file further @geoffw0 🙇🏻‍♀️

[paldepind]

Looks really good!

[paldepind]

I'm not sure I follow this. Since the sink here uses getStaticTarget it only works when we can resolve a target. Wouldn't we then also be able to find the MaD in the same cases as MaD works by finding the target and checking its canonical name?

[paldepind]

Would it make sense to also have tainted user input as a sink? In such cases a user could make the value true?

[geoffw0]

Yep, makes sense - I'm not sure if it will add many more results in practice but I'll add taint sources as a flow source here.

[geoffw0]

Done. It was a bit tricky to test as we didn't have any (direct) bool taint sources until now, but we've ended up with a bunch of new models and a second change note coming from that. I'll do a second DCA run as well - I think the effect on results for this query is going to be quite marginal, but best to check...