-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Implement new query for non-HTTPS URLs (CWE-319) #20432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: geoffw0 <[email protected]>
I've reviewed the initial version of this query by Copilot - it is complete, working, and to a high standard. I've added a few commits to expand the tests, address a few edge cases, and tweak the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a new CodeQL query for detecting the use of non-HTTPS URLs in Rust code, addressing CWE-319 (Cleartext Transmission of Sensitive Information). The query identifies HTTP URLs that could be intercepted by third parties and recommends using HTTPS for secure communication.
- Adds a taint tracking query that detects HTTP string literals and tracks their flow to network request functions
- Implements proper filtering to exclude localhost and private IP addresses to avoid false positives
- Provides comprehensive test coverage and documentation with examples of vulnerable and secure patterns
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
rust/ql/src/queries/security/CWE-319/UseOfHttp.ql | Main query implementing taint tracking for HTTP URLs |
rust/ql/lib/codeql/rust/security/UseOfHttpExtensions.qll | Library defining sources (HTTP literals) and sinks (request URLs) |
rust/ql/src/queries/security/CWE-319/UseOfHttp.qhelp | Documentation explaining the vulnerability with examples |
rust/ql/src/queries/security/CWE-319/UseOfHttpBad.rs | Example demonstrating vulnerable HTTP usage |
rust/ql/src/queries/security/CWE-319/UseOfHttpGood.rs | Example demonstrating secure HTTPS usage |
rust/ql/test/query-tests/security/CWE-319/main.rs | Comprehensive test cases covering various HTTP URL patterns |
rust/ql/test/query-tests/security/CWE-319/options.yml | Test configuration with reqwest dependency |
rust/ql/test/query-tests/security/CWE-319/UseOfHttp.qlref | Query reference for test execution |
rust/ql/test/query-tests/security/CWE-319/UseOfHttp.expected | Expected test results showing detected HTTP URLs |
rust/ql/src/queries/summary/Stats.qll | Updates to include new query in statistics |
rust/ql/src/change-notes/2025-09-15-non-https-url.md | Change note documenting the new query |
rust/ql/integration-tests/query-suite/*.qls.expected | Integration test updates adding query to security suites |
exists(string s | this.getTextValue() = s | | ||
// Match HTTP URLs that are not private/local | ||
s.regexpMatch("\"http://.*\"") and | ||
not s.regexpMatch("\"http://(localhost|127\\.0\\.0\\.1|192\\.168\\.[0-9]+\\.[0-9]+|10\\.[0-9]+\\.[0-9]+\\.[0-9]+|172\\.16\\.[0-9]+\\.[0-9]+|\\[::1\\]|\\[0:0:0:0:0:0:0:1\\]).*\"") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IPv4 private network ranges are incomplete. The regex is missing the 172.16.0.0/12 range (should be 172\.(1[6-9]|2[0-9]|3[01])\.) and the 169.254.0.0/16 link-local range. Also, IPv6 private ranges like fc00::/7 and fe80::/10 are not covered.
not s.regexpMatch("\"http://(localhost|127\\.0\\.0\\.1|192\\.168\\.[0-9]+\\.[0-9]+|10\\.[0-9]+\\.[0-9]+\\.[0-9]+|172\\.16\\.[0-9]+\\.[0-9]+|\\[::1\\]|\\[0:0:0:0:0:0:0:1\\]).*\"") | |
not s.regexpMatch("\"http://(localhost|127\\.0\\.0\\.1|192\\.168\\.[0-9]+\\.[0-9]+|10\\.[0-9]+\\.[0-9]+\\.[0-9]+|172\\.(1[6-9]|2[0-9]|3[01])\\.[0-9]+\\.[0-9]+|169\\.254\\.[0-9]+\\.[0-9]+|\\[::1\\]|\\[0:0:0:0:0:0:0:1\\]|\\[fc[0-9a-fA-F]{2}:.*\\]|\\[fd[0-9a-fA-F]{2}:.*\\]|\\[fe80:.*\\]).*\"") |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IPv4 one has already been added in the latest version, I'll look into the IPv6 ranges as well...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... I've added the IPv6 private address range. 169.254.x.x and fe80::/10 appear to be something slightly different - "link-local addresses" - and my knowledge of IP doesn't extend to whether we should include those in this logic. I suspect it isn't that important (unless we start seeing lots of false positive results about them).
QHelp previews: rust/ql/src/queries/security/CWE-319/UseOfHttp.qhelpFailure to use HTTPS URLsConstructing URLs with the HTTP protocol can lead to insecure connections. Furthermore, constructing URLs with the HTTP protocol can create problems if other parts of the code expect HTTPS URLs. A typical pattern is to use libraries that expect secure connections, which may fail or fall back to insecure behavior when provided with HTTP URLs instead of HTTPS URLs. RecommendationWhen you construct a URL for network requests, ensure that you use an HTTPS URL rather than an HTTP URL. Then, any connections that are made using that URL are secure TLS connections. ExampleThe following examples show two ways of making a network request using a URL. When the request is made using an HTTP URL rather than an HTTPS URL, the connection is unsecured and can be intercepted by attackers: // BAD: Using HTTP URL which can be intercepted
use reqwest;
fn main() {
let url = "http://example.com/sensitive-data";
// This makes an insecure HTTP request that can be intercepted
let response = reqwest::blocking::get(url).unwrap();
println!("Response: {}", response.text().unwrap());
} A better approach is to use HTTPS. When the request is made using an HTTPS URL, the connection is a secure TLS connection: // GOOD: Using HTTPS URL which provides encryption
use reqwest;
fn main() {
let url = "https://example.com/sensitive-data";
// This makes a secure HTTPS request that is encrypted
let response = reqwest::blocking::get(url).unwrap();
println!("Response: {}", response.text().unwrap());
} References
|
MRVA - mix of correct results, and results in tests (which are correct but not interesting). 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments, but otherwise this looks great.
} | ||
|
||
// Additional test cases that mirror the Bad/Good examples | ||
fn test_examples() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the tests in this function testing something that's not already tested above? Could we just delete these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're the examples from the .qhelp
, so we're testing that the code in the examples is correct (compiles) and is found by the query. Ideally this would be automated, there's nothing stopping the two copies diverging in future, but it's better than risking publishing an example that doesn't compile IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No further comments from Docs—looks good 👍
Co-authored-by: Simon Friis Vindum <[email protected]>
This PR implements a new CodeQL query for detecting the use of non-HTTPS URLs in Rust code, addressing CWE-319 (Cleartext Transmission of Sensitive Information).
Overview
The query detects when Rust code may be accessing HTTP (rather than HTTPS) URLs, which is a security vulnerability as these connections can be intercepted by third parties. This follows the pattern of existing non-HTTPS URL queries in other languages like Java (
java/non-https-url
) and C++ (cpp/non-https-url
).Implementation Details
Query Components
Key Features
"http://example.com/api"
and"http://"
request-url
sinks from models-as-data to identify where URLs are used in network requestsreqwest
through existing modelsTest Coverage
The test suite verifies detection of:
reqwest::blocking::get("http://example.com/api")
format!("{}{}", "http://", host)
http://127.0.0.1:3000
The query successfully detects 5 different HTTP URL patterns in the test cases while correctly excluding private/local addresses.
Files Added
rust/ql/src/queries/security/CWE-319/UseOfHttp.ql
rust/ql/lib/codeql/rust/security/UseOfHttpExtensions.qll
rust/ql/src/queries/security/CWE-319/UseOfHttp.qhelp
rust/ql/src/queries/security/CWE-319/UseOfHttpBad.rs
rust/ql/src/queries/security/CWE-319/UseOfHttpGood.rs
rust/ql/test/query-tests/security/CWE-319/
The query has
@precision high
and is included in all relevant query suites for security scanning.Fixes #20417
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.