Go: Support for MaD barriers and barrier guards.#21015
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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
aschackmull
force-pushed
the
go/mad-barriers
branch
2 times, most recently
from
70bdb01 to
66ec419
Compare
aschackmull
force-pushed
the
go/mad-barriers
branch
from
66ec419 to
10a0530
Compare
owen-mc
force-pushed
the
go/mad-barriers
branch
from
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). |
owen-mc
force-pushed
the
go/mad-barriers
branch
from
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. |
owen-mc
force-pushed
the
go/mad-barriers
branch
from
82e1eb4 to
d9d12cf
Compare
owen-mc
force-pushed
the
go/mad-barriers
branch
from
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.
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. |
|
@owen-mc your changes LGTM. Should we merge? |
2 participants
This adds models-as-data support for Go barriers and barrier guards.