Skip to content

Rust: Add neutral models for spurious Vec and VecDeque sink models - #22871

Merged
geoffw0 merged 1 commit into
github:mainfrom
LucaCappelletti94:upstream/rust-memory-queue-logging
Oct 22, 2026
Merged

geoffw0 merged 1 commit into
github:mainfrom
LucaCappelletti94:upstream/rust-memory-queue-logging

Conversation

@LucaCappelletti94

Copy link
Copy Markdown
Contributor

The generated standard library models mark VecDeque::push_back, append, insert, reserve and 18 other VecDeque methods, together with Vec::insert, Vec::remove and Vec::swap_remove, as log-injection sinks. They appear to come from the panic and out-of-memory messages inside those methods that print a length, capacity or index. As a result rust/cleartext-logging reports queue.push_back(x) as a write to a log file whenever queue is tainted by sensitive data.

This adds neutral sink models for those methods next to the existing split_off and From::from ones, as in #21375. The neutral rows mirror the generated rows one for one, and every method concerned has only log-injection sink models, so no other sink kind is suppressed.

@LucaCappelletti94
LucaCappelletti94 requested a review from a team as a code owner October 22, 2026 08:28
Copilot AI max review requested due to automatic review settings October 22, 2026 08:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added documentation Rust Pull requests that update Rust code external-contribution labels Oct 22, 2026

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is the right approach and the models LGTM. Test cases prove they work. I've started a DCA run (additional CI), which I suspect will show a few real-world FP results disappearing.

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. We see a quite significant reduction in false positive results for the cleartext logging query.

Thank you for your contribution! 🚀

@geoffw0
geoffw0 merged commit 979ae67 into github:main Oct 22, 2026
17 checks passed
CommanderStorm pushed a commit to maplibre/martin that referenced this pull request Oct 25, 2026
CodeQL's request-taint queries see nothing inside the 12 route handlers
that `#[hotpath::measure]` instruments, the tile, TileJSON, catalog,
font, sprite and style handlers among them. Its Rust extractor turns on
every Cargo feature unless told otherwise
([docs](https://github.com/github/codeql/blob/codeql-cli/v2.27.3/rust/codeql-extractor.yml#L48-L54)),
so hotpath's profiling is on, and the attribute then hands each async
body to `hotpath::functions::measure_async` as an `async` block
([macro](https://github.com/hotpath-rs/hotpath/blob/4eb2e398d4a3bab902faa1809e6cfca48c1e919e/crates/hotpath-macros/src/lib_on.rs#L861-L864))
that CodeQL can't follow into. The CodeQL job now gives the extractor
every workspace feature except the hotpath ones, read from `cargo
metadata`, the way `just check` already leaves them out of its feature
matrix.

Seeing into those handlers, CodeQL flags their `req.connection_info()`
calls, as it did [the OGC one on
#3763](#3763 (comment)):
its generated actix-web model makes `connection_info` an allocation and
log sink, though it only reads the host, scheme and client address off
the request. A neutral model in `.github/codeql/extensions`, which [code
scanning loads on its
own](https://docs.github.com/en/code-security/how-tos/find-and-fix-code-vulnerabilities/manage-your-configuration/edit-default-setup#extending-coverage-for-a-repository),
cancels those generated sinks the way github/codeql#22871 does for
`Vec`, until github/codeql#22884 ships the same rows upstream. With CI's
suite and CodeQL 2.27.3, main goes from 7 results to 6, the OGC one
included.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation external-contribution Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants