-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Go: Support for MaD barriers and barrier guards. #21015
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
base: main
Are you sure you want to change the base?
Conversation
| pragma[noinline] | ||
| private predicate guards(Node g, ControlFlow::ConditionGuardNode guard, Node nd, SsaWithFields ap) { | ||
| guards(g, guard, nd) and nd = ap.getAUse() | ||
| private predicate guards( |
Check warning
Code scanning / CodeQL
Missing QLDoc for parameter Warning
70bdb01 to
66ec419
Compare
66ec419 to
10a0530
Compare
10a0530 to
0539b18
Compare
|
Now that #21011 is merged I've rebased this and added commits to convert all the easily-convertible barriers and barrier guards to MaD. After running DCA I think this could be merged (without a change note). |
0539b18 to
82e1eb4
Compare
|
@aschackmull I rebased this and added some commits converting the go barriers to MaD. I think this is ready to review, but I'll leave it to you to press the button. |
82e1eb4 to
d9d12cf
Compare
d9d12cf to
8602c4c
Compare
|
@aschackmull I've updated this to remove query-specific MaD sanitizers and implement them for all the sink kinds that go currently uses. This means some existing sanitizers aren't converted, but that's okay. I consider this ready for review and DCA. Note that I don't think the validation of sanitizer kinds is working, as when I was using query ids there weren't any test failures. |
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 pull request adds Models-as-Data (MaD) support for Go barriers and barrier guards, enabling external specification of sanitizers and barrier guards through data models.
Key changes:
- Added infrastructure for barrier and barrier guard models in the data flow framework
- Introduced
ParameterizedBarrierGuardmodule to support parameterized barrier guards - Migrated several hardcoded sanitizer classes to external MaD models
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
go/ql/lib/semmle/go/dataflow/ExternalFlow.qll |
Added barrierNode predicate and barrier guard checking logic to support MaD barriers |
go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll |
Implemented barrierElement and barrierGuardElement predicates for MaD integration |
go/ql/lib/semmle/go/dataflow/internal/DataFlowUtil.qll |
Added ParameterizedBarrierGuard module and parameterized existing BarrierGuard module |
go/ql/lib/semmle/go/frameworks/XPath.qll |
Added new Sanitizer abstract class and external sanitizer implementation |
go/ql/lib/semmle/go/frameworks/stdlib/Regexp.qll |
Renamed classes to use "External" prefix for MaD-based implementations |
go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll |
Renamed to "External" prefix for consistency with MaD pattern |
go/ql/lib/semmle/go/frameworks/SQL.qll |
Renamed to "External" prefix for consistency with MaD pattern |
go/ql/lib/semmle/go/frameworks/NoSQL.qll |
Renamed to "External" prefix for consistency with MaD pattern |
go/ql/lib/semmle/go/frameworks/Beego.qll |
Removed hardcoded HtmlQuoteSanitizer class (migrated to MaD model) |
go/ql/lib/semmle/go/security/Xss.qll |
Renamed sink class and added external sanitizer for XSS vulnerabilities |
go/ql/lib/semmle/go/security/XPathInjectionCustomizations.qll |
Added XPathSanitizer class to integrate with external models |
go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll |
Added external sanitizer and removed hardcoded sanitizer classes (migrated to MaD) |
go/ql/lib/semmle/go/security/SqlInjectionCustomizations.qll |
Added external sanitizer for SQL injection |
go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll |
Renamed sink class and added external sanitizer |
go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll |
Added external barrier for URL redirection |
go/ql/lib/semmle/go/security/MissingJwtSignatureCheckCustomizations.qll |
Renamed sink class and added external sanitizer |
go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll |
Added isBarrier predicate to support sanitizers |
go/ql/lib/semmle/go/security/CommandInjectionCustomizations.qll |
Added external sanitizer for command injection |
go/ql/lib/semmle/go/security/HardcodedCredentials.qll |
Renamed sink class and added external credentials sanitizer |
go/ql/lib/semmle/go/concepts/HTTP.qll |
Renamed to "External" prefix for consistency with MaD pattern |
go/ql/lib/semmle/go/Concepts.qll |
Renamed classes to "External" prefix for consistency with MaD pattern |
go/ql/lib/ext/path.filepath.model.yml |
Added barrier model for filepath.Rel function |
go/ql/lib/ext/mime.multipart.model.yml |
Added barrier models for FileHeader.Filename field and Part.FileName method |
go/ql/lib/ext/github.com.beego.beego.server.web.model.yml |
Added barrier model for Htmlquote function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I think the corresponding qltest to reference the model validation is simply missing, for Java it's e.g. https://github.com/github/codeql/blob/main/java/ql/test/library-tests/dataflow/external-models/validatemodels.ql and similar tests exist for C++ and C#. |
|
I verified on my machine that the barrier models are now validated for at least the namespace and kind columns. We don't have a separate query for it, but some (all?) of the existing queries fail. |
This adds models-as-data support for Go barriers and barrier guards.