-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Model mysql and mysql_async sources #20634
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
Conversation
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 adds support for modeling taint sources in the MySQL and mysql_async Rust libraries. It extends the existing CodeQL Rust analysis to identify database query results as taint sources, which is important for security analysis of applications using these database libraries.
- Adds comprehensive taint source modeling for MySQL query methods and result accessors
- Includes flow models for propagating taint through query result processing functions
- Provides extensive test coverage for both synchronous and asynchronous MySQL operations
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/library-tests/dataflow/sources/test.rs | Adds comprehensive test cases for mysql and mysql_async taint sources and data flow |
| rust/ql/test/library-tests/dataflow/sources/options.yml | Adds mysql and mysql_async dependencies for testing |
| rust/ql/test/library-tests/dataflow/sources/TaintSources.expected | Updates expected test results with new database taint sources |
| rust/ql/test/library-tests/dataflow/sources/CONSISTENCY/PathResolutionConsistency.expected | Updates path resolution consistency test expectations |
| rust/ql/lib/codeql/rust/frameworks/mysql.model.yml | Adds source and summary models for mysql library taint tracking |
| rust/ql/lib/codeql/rust/frameworks/mysql-async.model.yml | Adds source and summary models for mysql_async library taint tracking |
|
DCA LGTM. |
paldepind
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.
I've only done a lightweight review. But from a quick look, this LGTM!
| - futures-rustls = { version = "0.26.0" } | ||
| - async-std = { version = "1.13.1" } | ||
| - warp = { version = "0.4.2", features = ["server"] } | ||
| - mysql = { version = "26.0.1" } |
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.
I don't know what your experience is @geoffw0, but I've felt that extracting the "sources" project is very slow.
I think it's due to the number of dependencies. Maybe we could consider adding separate directories going forward with specific libraries. That would eliminate this single project that just keeps on growing.
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.
Yes, its crossed a line at some point from a slight irritation to a genuine time sink. I plan to split it into three or four parts (according to libraries used) just as soon as this PR is merged!
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.
Here: #20673
Model mysql and mysql_async taint sources, and some flow as well. Follows #20631 , and the change note from that covers these changes as well.