Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 30, 2025

Adds barriers to reduce false positives in the rust/sql-injection query by recognizing common sanitization patterns.

Changes

Barriers Implemented

  • NumericTypeBarrier: Blocks taint when data is parsed to numeric types (i8-i128, u8-u128, f32, f64, isize, usize) via .parse::<Type>()
  • StringConstCompareBarrier: Blocks taint in branches where data is validated against string literal(s) using ==/!= or logical OR combinations
  • StringConstArrayInclusionCallBarrier: Blocks taint when data is validated via .contains() on array/slice of string literals

Test Coverage

Added barriers.rs with test cases covering:

  • Numeric type conversions (should not alert)
  • Single and multiple constant comparisons (should not alert in validated branches)
  • Array membership checks (should not alert in validated branches)
  • Negative cases for incorrect sanitization (should still alert)

Example

let remote_string = get_remote_input();

// Now recognized as safe - numeric barrier
let id = remote_string.parse::<i32>().unwrap();
let query = format!("SELECT * FROM users WHERE id={}", id);

// Now recognized as safe in this branch - comparison barrier
if remote_string == "admin" || remote_string == "user" {
    let query = format!("SELECT * FROM users WHERE role='{}'", remote_string);
}

// Now recognized as safe in this branch - array inclusion barrier
if ["alice", "bob"].contains(&remote_string.as_str()) {
    let query = format!("SELECT * FROM users WHERE name='{}'", remote_string);
}

Implementation follows patterns from Go's SimpleTypeSanitizer and Ruby's StringConstCompareBarrier/StringConstArrayInclusionCallBarrier, adapted for Rust's control flow graph and barrier guard mechanism.

Original prompt

This section details on the original issue you should resolve

<issue_title>Rust: Add barriers for rust/sql-injection</issue_title>
<issue_description>Add taint flow barriers to the rust/sql-injection CodeQL query. This query detects SQL injection vulnerabilities, by means of taint flow from a source where the program reads untrusted data, to a sink where that data is used to construct an SQL query (untrusted prepared query parameters are of course fine).

We would like to add barriers to block taint flow when one of the following types of sanitization are present:

  • when the untrusted data is restricted to a numeric type.
  • when the untrusted data is compared with a single constant value to confirm it is safe.
  • when the untrusted data is compared with multiple constant values to confirm it is safe (for example if (remote_string == "person") or (remote_string == "vehicle")).
  • when the untrusted data is compared against a collection of constant values to confirm it is safe.

There's already an example of the first case in the tests (safe_query_3 defined in terms of remote_number in the sqlx.rs test source). The first and most important part of this task will be to add test cases covering the other three cases above, plus a couple of "negative" examples where sanitization is performed incorrectly.

Some useful locations for you:

  • the query can be found at rust/ql/src/queries/security/CWE-089/SqlInjection.ql.
  • its tests are in rust/ql/test/query-tests/security/CWE-089/*.rs.
  • the query extensions are in rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll.

The second part of the task is to add barriers to the query, or more precisely to the "Extensions" file for the query. You may want to base your solution on:

  • SimpleTypeSanitizer in go/ql/lib/semmle/go/security/Sanitizers.qll.
  • StringConstCompareBarrier in ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll.
  • StringConstArrayInclusionCallBarrier in ruby/ql/lib/codeql/ruby/dataflow/BarrierGuards.qll.
  • UpperBoundCheckBarrier in rust/ql/lib/codeql/rust/security/UncontrolledAllocationSizeExtensions.qll (this one seems slightly less relevant, but the target language is Rust so some of the syntax may be closer to what you actually need).

If you struggle to get these working, do the best you can and I'll help fix it up after. As long as we have good test cases we should be in a good position to do this.

I prefer the phrase "Barrier" over "Sanitizer" in new code.


When the work is done, I request that you complete a few final cleanup / admin tasks:

  • run the CodeQL CLI autoformatter on all .ql and .qll files you have created or edited. With the CodeQL CLI installed: codeql query format [file] --in-place.
  • add a brief change note file in rust/ql/src/change-notes. See https://github.com/github/codeql/blob/main/docs/change-notes.md for more information about change notes.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ 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 AI and others added 3 commits October 30, 2025 16:15
Co-authored-by: geoffw0 <40627776+geoffw0@users.noreply.github.com>
Co-authored-by: geoffw0 <40627776+geoffw0@users.noreply.github.com>
Co-authored-by: geoffw0 <40627776+geoffw0@users.noreply.github.com>
Copilot AI changed the title [WIP] Add barriers for rust/sql-injection CodeQL query Add taint flow barriers to rust/sql-injection query Oct 30, 2025
Copilot AI requested a review from geoffw0 October 30, 2025 16:51
Copilot finished work on behalf of geoffw0 October 30, 2025 16:51
@geoffw0
Copy link
Contributor

geoffw0 commented Oct 31, 2025

I started making some edits to this, but quickly realised there's too much that needs changing to be a practical starting point. So the plan now is to implement this the old fashioned way, very likely taking some of the good bits from here as I go.

Closing.

@geoffw0 geoffw0 closed this Oct 31, 2025
@geoffw0
Copy link
Contributor

geoffw0 commented Oct 31, 2025

Manual work for the numeric barrier here: #20740

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rust: Add barriers for rust/sql-injection

2 participants