Skip to content

Rust: Add neutral models (map, from)#21375

Open
geoffw0 wants to merge 10 commits intogithub:mainfrom
geoffw0:mapfix
Open

Rust: Add neutral models (map, from)#21375
geoffw0 wants to merge 10 commits intogithub:mainfrom
geoffw0:mapfix

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Feb 26, 2026

Add neutral models for particular versions of map and from, where we have spurious generated sink models that are a common cause of false positive results.

The exact variations (i.e. the data in extensible: neutralModel in core.model.yml) is a bit of an untidy list, because adding a neutral model for <_ as core::convert::From>::from doesn't work at present. I'm sure we'll want to refine it later, it's also possible we may want to alter the model generator itself. Nevertheless results in the tests are good, and I expect to see a significant reduction in real world false positive results for rust/uncontrolled-allocation-size and rust/log-injection (DCA should confirm).

I've also added a missing flow model for <core::option::Option>::map, that was exposed by one of the test cases.

@geoffw0 geoffw0 added the Rust Pull requests that update Rust code label Feb 26, 2026
@geoffw0 geoffw0 requested a review from a team as a code owner February 26, 2026 11:37
Copilot AI review requested due to automatic review settings February 26, 2026 11:37
Copy link
Contributor

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

Adds/adjusts Rust CodeQL modeling to reduce false positives from spurious generated sink models around map and from, and updates/extends security query tests to validate the new behavior.

Changes:

  • Add manual flow summary models for <core::option::Option>::map and add neutral models to suppress generated sink models for selected map/From::from implementations.
  • Extend CWE-770 and CWE-117 tests to cover the new modeling (including an axum-based scenario) and update expected outputs/lockfile.
  • Add a changenote describing the analysis impact.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rust/ql/test/query-tests/security/CWE-770/main.rs Adds a regression test ensuring Vec::from/From::from aren’t treated as allocation-size sinks while keeping malloc(a) flagged.
rust/ql/test/query-tests/security/CWE-770/UncontrolledAllocationSize.expected Updates expected results for the modified CWE-770 test.
rust/ql/test/query-tests/security/CWE-117/options.yml Adds axum dependency for new log-injection test coverage.
rust/ql/test/query-tests/security/CWE-117/main.rs Adds non-sink From::from conversions and an axum routing/Option::map scenario to exercise new models.
rust/ql/test/query-tests/security/CWE-117/LogInjection.expected Updates expected results/models to reflect new Option::map summary and axum source modeling.
rust/ql/test/query-tests/security/CWE-117/Cargo.lock Updates lockfile to include axum (and related dependency resolution changes).
rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml Adds Option::map flow summaries and adds neutralModel entries to suppress generated sink models for listed functions/impls.
rust/ql/lib/change-notes/2026-02-26-neutral-models-map-from.md Adds a changenote for the analysis/modeling change.

Comment on lines +106 to +107
- ["<core::option::Option>::map", "Argument[self]", "Argument[0].Parameter[0]", "taint", "manual"]
- ["<core::option::Option>::map", "Argument[0].ReturnValue", "ReturnValue", "taint", "manual"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have

- ["<core::option::Option>::map", "Argument[0].ReturnValue", "ReturnValue.Field[core::option::Option::Some(0)]", "value", "dfc-generated"]

so I think what we are missing is

- ["<core::option::Option>::map", "Argument[self].Field[core::option::Option::Some(0)]", "Argument[0].Parameter[0]", "value", "manual"]

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

Labels

documentation Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants