Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Jan 12, 2026

Wrote some logic for identifying missing .Reference tokens, which is also included with this PR.

DCA is fine.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jan 12, 2026
private import codeql.rust.internal.TypeMention
private import codeql.rust.internal.Type

private predicate relevantManualModel(SummarizedCallableImpl sc, string can) {

Check warning

Code scanning / CodeQL

Dead code Warning

This code is never used, and it's not publicly exported.
)
}

predicate manualModelMissingReturnReference(

Check warning

Code scanning / CodeQL

Dead code Warning

This code is never used, and it's not publicly exported.
@hvitved hvitved force-pushed the rust/fix-more-models branch 3 times, most recently from ad07fbf to edf0dea Compare January 12, 2026 13:00
@hvitved hvitved force-pushed the rust/fix-more-models branch from edf0dea to 17441a5 Compare January 12, 2026 13:22
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Jan 12, 2026
@hvitved hvitved marked this pull request as ready for review January 12, 2026 13:22
@hvitved hvitved requested a review from a team as a code owner January 12, 2026 13:22
@hvitved hvitved requested review from Copilot and geoffw0 January 12, 2026 13:22
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

This PR corrects data flow models for Rust standard library functions by adding missing .Reference tokens where parameters or return values are reference types. The changes also include debug logic to automatically detect such missing references in future manual models.

Changes:

  • Updated 9 data flow models across 5 YAML files to properly handle reference types
  • Added debug predicates to detect missing .Reference tokens in manual models
  • Updated test expectations to reflect the corrected models
  • Fixed a test case that now correctly detects taint flow through BufReader::lines()

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rust/ql/lib/codeql/rust/frameworks/stdlib/alloc.model.yml Updated String::as_str and String::as_bytes to include .Reference tokens; removed redundant Argument[self,0] model for String::add
rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml Updated str::as_str to include .Reference tokens and changed kind from "value" to "taint"
rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml Updated Path::join to add .Reference to Argument[self]
rust/ql/lib/codeql/rust/frameworks/stdlib/net.model.yml Updated TcpStream::try_clone to add .Reference to Argument[self]
rust/ql/lib/codeql/rust/frameworks/futures.model.yml Removed duplicate model for AsyncReadExt::read
rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll Added debug module with predicates to detect missing .Reference tokens; refactored summaryModelRelevant predicate
rust/ql/test/library-tests/dataflow/sources/net/test.rs Updated comment on line 207 from MISSING: to reflect fixed test case
*.expected files Updated test expectations to reflect corrected model numbers and additional flow paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Happy with this, thanks for including the debug code so we can check for issues like these again in future.

- ["<_ as alloc::string::ToString>::to_string", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
# Overwrite generated model
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[self,0]", "ReturnValue", "taint", "manual"]
- ["<alloc::string::String as core::ops::arith::Add>::add", "Argument[self]", "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.

I thought there was some kind of edge case behind the overlapping .Reference and non-.Reference models here. Something to do with appending &str strings perhaps, I can't remember exactly. Still, as this stuff is well tested and I see no regressions in the tests, I'm happy.

Alternative possible explanation: the incorrect models were only required for compatibility with other incorrect models elsewhere, which you've already updated.

@hvitved hvitved merged commit c666fc7 into github:main Jan 12, 2026
28 checks passed
@hvitved hvitved deleted the rust/fix-more-models branch January 12, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants