Ruby: Accept MaD sanitizers for queries with MaD sinks and convert some existing sanitizers#21341
Open
owen-mc wants to merge 16 commits intogithub:mainfrom
Open
Ruby: Accept MaD sanitizers for queries with MaD sinks and convert some existing sanitizers#21341owen-mc wants to merge 16 commits intogithub:mainfrom
owen-mc wants to merge 16 commits intogithub:mainfrom
Conversation
Note that some sanitizers had no effect because flow through those functions wasn't modeled.
Note that this will only block flow for queries that use the kind `command-injection`.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enables Ruby security queries to accept MaD (Models as Data) sanitizers and converts several existing QL-based sanitizers to MaD models. The change improves consistency across the codebase and enables better reuse of sanitizer models. As a side benefit, Shellwords.escape and Shellwords.shellescape now propagate taint for all queries except command injection (where they act as sanitizers).
Changes:
- Added
ExternalXXXSanitizerclasses to six security query customization files to accept MaD barrier models - Converted three existing sanitizers (
SQLite3::Database.quote,Mysql2::Client.escape, andShellwords.escape/shellescape) from QL code to MaD models - Updated tests for sqlite3 and mysql2 frameworks with proper inline expectations and SQL injection query references
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll | Added ExternalUrlRedirectSanitizer to accept MaD barrier models for url-redirection |
| ruby/ql/lib/codeql/ruby/security/ServerSideRequestForgeryCustomizations.qll | Added ExternalRequestForgerySanitizer to accept MaD barrier models for request-forgery |
| ruby/ql/lib/codeql/ruby/security/PathInjectionCustomizations.qll | Added ExternalPathInjectionSanitizer to accept MaD barrier models for path-injection |
| ruby/ql/lib/codeql/ruby/security/LogInjectionQuery.qll | Added ExternalLogInjectionSanitizer to accept MaD barrier models for log-injection |
| ruby/ql/lib/codeql/ruby/security/CommandInjectionCustomizations.qll | Added ExternalCommandInjectionSanitizer; updated ShellwordsEscapeAsSanitizer to only handle String#shellescape instance method |
| ruby/ql/lib/codeql/ruby/security/CodeInjectionCustomizations.qll | Added ExternalCodeInjectionSanitizer to accept MaD barrier models for code-injection |
| ruby/ql/lib/codeql/ruby/Concepts.qll | Added ExternalSqlInjectionSanitizer to accept MaD barrier models for sql-injection; added import for ApiGraphModels |
| ruby/ql/lib/codeql/ruby/frameworks/Sqlite3.qll | Removed SQLite3QuoteSanitization class and QuoteSummary; now handled by MaD model |
| ruby/ql/lib/codeql/ruby/frameworks/Sqlite3.model.yml | Created MaD model with summary (taint flow) and barrier (sql-injection) for SQLite3::Database.quote |
| ruby/ql/lib/codeql/ruby/frameworks/Mysql2.qll | Removed Mysql2EscapeSanitization class and EscapeSummary; now handled by MaD model |
| ruby/ql/lib/codeql/ruby/frameworks/Mysql2.model.yml | Created MaD model with summary (taint flow) and barrier (sql-injection) for Mysql2::Client.escape |
| ruby/ql/lib/codeql/ruby/frameworks/stdlib/Shellwords.model.yml | Created MaD model with summary (taint flow) and barrier (command-injection) for Shellwords.escape and shellescape |
| ruby/ql/test/library-tests/frameworks/sqlite3/sqlite3.rb | Updated test with ActionController example demonstrating SQLite3::Database.quote as sanitizer |
| ruby/ql/test/library-tests/frameworks/sqlite3/SqlInjection.qlref | Created query reference file for SqlInjection test |
| ruby/ql/test/library-tests/frameworks/sqlite3/SqlInjection.expected | Generated expected results showing SQL injection detection and proper sanitization |
| ruby/ql/test/library-tests/frameworks/mysql2/Mysql2.rb | Added inline expectations for SQL injection sources and alerts |
| ruby/ql/test/library-tests/frameworks/mysql2/SqlInjection.qlref | Created query reference file for SqlInjection test |
| ruby/ql/test/library-tests/frameworks/mysql2/SqlInjection.expected | Generated expected results showing SQL injection detection with Mysql2::Client.escape as sanitizer |
| ruby/ql/lib/change-notes/2026-02-17-flow-through-shellwords-escape-shellescape.md | Documented that taint now flows through Shellwords.escape/shellescape for all queries except command injection |
hvitved
requested changes
Feb 18, 2026
Contributor
hvitved
left a comment
hvitved
left a comment
There was a problem hiding this comment.
Some tests are failing, otherwise LGTM. Also, remember to run DCA.
Need to do this because the model numbering was changing. At the same time we may as well use inline expectations.
Contributor
Author
|
Test results updated. DCA run. I haven't interpreted Ruby DCA runs before - the average analysis time looks fine, but there's quite a bit of spread, and I don't know if that's normal or not. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note that in the process of converting
Shellwords.escapeandShellwords.shellescape, I have added a summary model for them as well as a command injection sanitizer, so as an upshot they will now propagate taint for all other queries.