-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Regenerate models #21023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rust: Regenerate models #21023
Conversation
dacb222 to
9f37463
Compare
d41aba2 to
3cf602f
Compare
| predicate includeDynamicTargets(); | ||
| } | ||
|
|
||
| module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gen is for "generic" or something. Suggestions for better names are very welcome 🙏
There was a problem hiding this 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 regenerates auto-generated models for Rust by disabling dynamic dispatch during model generation. The change prevents problematic sinks from appearing when trait function implementations are dynamically dispatched (e.g., preventing parse from inheriting sinks from all FromStr implementations).
Changes:
- Adds a parameterized module system to control dynamic dispatch in data flow analysis
- Updates model generation to disable dynamic dispatch when generating models
- Regenerates models for multiple Rust libraries (serde, smallvec, once_cell, reqwest)
- Updates test expectations to reflect the new generated models
Reviewed changes
Copilot reviewed 16 out of 30 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ModelGeneratorImpl.qll | Adds applyReadStepsAsTaintSteps() predicate to control read step behavior |
| CaptureModels.qll | Creates parameterized modules for disabling dynamic dispatch, adds path filtering for semicolons |
| TaintTrackingImpl.qll | Converts to parameterized module to support dynamic dispatch control |
| Test expectations | Updates model references to match regenerated models |
| Generated models | Regenerates models with dynamic dispatch disabled |
| reqwest.model.yml | Adds exclusion for timeout field taint step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hvitved
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Now that we generate more accurate models, these two manual models can be removed.
| class DataFlowSecondLevelScope = Void; | ||
| } | ||
|
|
||
| module RustDataFlowInput implements RustDataFlowInputSig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used over in TaintTrackingImpl.qll.
| TClosureSelfParameterPosition() or | ||
| TSelfParameterPosition() | ||
|
|
||
| final class TParameterPosition = TParameterPositionImpl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this achieve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes a compile error on this line:
final class ParameterPosition extends TParameterPosition {with the error being:
classes may not extend outside their parameterised module; consider extending through a final alias
@michaelnebel : Do you know if we have seen something similar in Java or C#? |
I believe, we have seen issues like this for source generation, which we worked around by introducing the (poorly named) predicate |
3cf602f to
76a1e22
Compare
Nice, thanks! |
|
Thanks for doing this. I'm still not convinced we should be removing manual models when they are generated, as generated models are potentially more fragile / less trustworthy. Though the opposite argument is that they're easier to update when libraries change, and less prone to human error. I guess I don't feel we ever properly decided what the right approach is. That aside, I'm very pleased to see the model generator working again, and pleased to see a bunch of new results on DCA! 🚀 |
This PR regenerates the auto-generated models for Rust.
A few changes where necessary in order to re-run the model generator. Most of them should be understandable by the commit message, but "Disable dynamic dispatch when generating models" might require a bit more explanation:
The dynamic dispatch that we do for calls that resolve to trait functions caused some problematic sinks to pop up. As an example of where this happened consider
parse:from the standard library. This essentially just wraps
FromStr::from_strand dynamically dispatching at this call causesparseto get generated sinks from all implementations ofFromStr.To fix the problem this PR disables dynamic dispatch when running the model generator. This is achieved by creating a parameterized module around Rust's data flow library which allows one to disable dynamic dispatch. This feels a bit heavy-handed but actually didn't require that many changes and I can't think of another way of achieving the goal.