Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 15, 2025

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

  • UseOfHttp.ql: Main query with dataflow configuration to track HTTP URLs from sources to sinks
  • UseOfHttpExtensions.qll: Library defining sources (HTTP string literals) and sinks (request URLs)
  • UseOfHttp.qhelp: Documentation explaining the vulnerability and providing examples
  • UseOfHttpBad.rs / UseOfHttpGood.rs: Example code showing vulnerable and secure patterns

Key Features

  • Detects HTTP string literals like "http://example.com/api" and "http://"
  • Tracks dataflow from HTTP URL sources through string formatting and variable assignments
  • Uses existing request-url sinks from models-as-data to identify where URLs are used in network requests
  • Excludes localhost and private IP addresses to avoid false positives for local development
  • Works with popular HTTP libraries like reqwest through existing models

Test Coverage

The test suite verifies detection of:

  • Direct HTTP URL usage: reqwest::blocking::get("http://example.com/api")
  • Dynamic URL construction: format!("{}{}", "http://", host)
  • HTTP URLs passed through variables and string formatting
  • Proper exclusion of localhost URLs like 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
  • Test files in rust/ql/test/query-tests/security/CWE-319/
  • Change notes and integration test suite updates

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.

@Copilot Copilot AI changed the title [WIP] Rust: Implement a new query for non-HTTPs URLs Rust: Implement new query for non-HTTPS URLs (CWE-319) Sep 15, 2025
@Copilot Copilot AI requested a review from geoffw0 September 15, 2025 11:58
Copilot finished work on behalf of geoffw0 September 15, 2025 11:58
@github-actions github-actions bot added documentation Rust Pull requests that update Rust code labels Sep 16, 2025
@geoffw0
Copy link
Contributor

geoffw0 commented Sep 16, 2025

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 .qhelp in a couple of places. The next steps will be a DCA run, MRVA run, then code and docs reviews.

@geoffw0 geoffw0 marked this pull request as ready for review September 16, 2025 13:32
@geoffw0 geoffw0 requested a review from a team as a code owner September 16, 2025 13:32
@Copilot Copilot AI review requested due to automatic review settings September 16, 2025 13:32
Copy link
Contributor

@Copilot Copilot AI left a 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\\]).*\"")
Copy link
Preview

Copilot AI Sep 16, 2025

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.

Suggested change
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.

Copy link
Contributor

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...

Copy link
Contributor

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).

Copy link
Contributor

github-actions bot commented Sep 16, 2025

QHelp previews:

rust/ql/src/queries/security/CWE-319/UseOfHttp.qhelp

Failure to use HTTPS URLs

Constructing 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.

Recommendation

When 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.

Example

The 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

@geoffw0
Copy link
Contributor

geoffw0 commented Sep 17, 2025

MRVA - mix of correct results, and results in tests (which are correct but not interesting). 👍
DCA - nothing to see. 👍

@geoffw0 geoffw0 added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Sep 17, 2025
Copy link
Contributor

@paldepind paldepind left a 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() {
Copy link
Contributor

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?

Copy link
Contributor

@geoffw0 geoffw0 Sep 19, 2025

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.

subatoi
subatoi previously approved these changes Sep 19, 2025
Copy link
Contributor

@subatoi subatoi left a 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rust: Implement a new query for non-HTTPs URLs
4 participants