Rust: Model mysql and mysql_async query sinks#20631
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds SQL injection sink models for the mysql and mysql_async Rust libraries to improve security analysis coverage.
- Introduces sink models for various query methods in both sync and async MySQL libraries
- Includes comprehensive test coverage demonstrating SQL injection detection
- Updates documentation to reflect the newly supported frameworks
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/test/query-tests/security/CWE-089/options.yml | Adds mysql and mysql_async dependencies for testing |
| rust/ql/test/query-tests/security/CWE-089/mysql.rs | Test file demonstrating both safe and unsafe MySQL query patterns |
| rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected | Expected test results showing proper SQL injection detection |
| rust/ql/lib/codeql/rust/frameworks/mysql.model.yml | Sink models for synchronous mysql library query methods |
| rust/ql/lib/codeql/rust/frameworks/mysql-async.model.yml | Sink models for asynchronous mysql_async library query methods |
| rust/ql/lib/change-notes/2025-10-10-mysql.md | Change note documenting the new library support |
| docs/codeql/reusables/supported-frameworks.rst | Documentation update listing the new supported frameworks |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
DCA LGTM. |
|
|
||
| // prepared queries (safe) | ||
| let stmt = conn.prep(prepared_query.as_str())?; // $ sql-sink | ||
| let _ : Vec<i64> = conn.exec(&stmt, (remote_string.as_str(),))?; |
There was a problem hiding this comment.
What are these commas here and below?
here
v
let _ : Vec<i64> = conn.exec(&stmt, (remote_string.as_str(),))?;There was a problem hiding this comment.
It's one of those funny corners of languages - this is required to disambiguate a tuple with one element from ordinary parenthesis in Rust. See here.
Co-authored-by: Simon Friis Vindum <paldepind@github.com>
I'm generally against formatting tests, at least uniformly, as we should aim to have a high diversity of code in tests and auto formatting reduces it. I'm happy to address specific areas of confusion, and perhaps even format whole files in certain cases - but I don't want us to get anywhere close to this being a requirement (in tests). |
|
I have formatted just this test file. |
|
Thanks. From my point of view, I find auto-formatted code nicer to read and since formatting mostly only affect parsing, and rust_analyzer handles parsing, I'd think it wouldn't be something we'd have to test much. But that's just my preference :) |
True. It's probably fine if 90% of tests are formatted for ease of reading. |
|
@paldepind would you mind re-approving this? |
|
Thanks! |
2 participants
Model
mysqlandmysql_asyncquery sinks (sources will follow in another PR).