Rust: More test cases for add, sub etc #21191
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds additional test cases for Rust taint dataflow analysis to cover more arithmetic and bitwise operators. The tests reveal issues with call resolution when explicit type annotations are missing. The PR also consolidates operator models to reduce duplication.
Changes:
- Added
more_ops()test function covering negation, not, subtraction, multiplication, shift, and XOR operators - Added
std_ops()test function for explicit method calls to operator traits with MISSING annotations documenting known resolution issues - Added
wrappingmodule to test operations onWrapping<i64>types - Consolidated operator models in core.model.yml to use
Argument[self,0]instead of separate entries
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rust/ql/test/library-tests/dataflow/taint/main.rs | Added test functions for various operators and their method forms, including cases that reveal call resolution issues |
| rust/ql/test/library-tests/dataflow/taint/inline-taint-flow.expected | Auto-generated test expectations updated to reflect new test cases |
| rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected | Auto-generated test expectations updated to reflect new test cases |
| rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml | Consolidated operator models to reduce duplication by using combined argument specifications |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| fn std_ops() { | ||
| sink(source(1).add(2i64)); // $ hasTaintFlow=1 | ||
| sink(source(1).add(2)); // $ MISSING: hasTaintFlow=1 --- the missing results here are due to failing to resolve targets for `add` etc where there's no explicit type |
There was a problem hiding this comment.
This is a known issue; this is because 2 is currently assigned the type i32 instead of i64.
| a.add_assign(Wrapping(crate::source(3))); | ||
| a += source(4); | ||
| a += std::num::Wrapping(crate::source(5)); | ||
| sink(a); // $ hasTaintFlow=2 hasTaintFlow=4 MISSING: hasTaintFlow=3 hasTaintFlow=5 --- we don't currently find any `Call`s for `Wrapping` above |
There was a problem hiding this comment.
I believe this is because we need special models like core::num::wrapping::Wrapping as core::ops::arith::AddAssign for Wrapping. Something like
- ["<_ as core::ops::arith::AddAssign>::add_assign", "Argument[self].Reference.Field[core::num::wrapping::Wrapping(0)]", "Argument[self].Reference.Field[core::num::wrapping::Wrapping(0)]", "taint", "manual"]
- ["<_ as core::ops::arith::AddAssign>::add_assign", "Argument[0].Field[core::num::wrapping::Wrapping(0)]", "Argument[self].Reference.Field[core::num::wrapping::Wrapping(0)]", "taint", "manual"]There was a problem hiding this comment.
I'll try that, thanks...
2 participants
Left over work from last year. I thought we were missing some simple models I could easily add, but it turns out the test cases reveal deeper issues perhaps in call resolution.
@hvitved FYI