Skip to content

Conversation

@aschackmull
Copy link
Contributor

@aschackmull aschackmull commented Dec 11, 2025

This adds models-as-data support for Go barriers and barrier guards.

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

The QLDoc has no documentation for param, but the QLDoc mentions getAGuardedNode
@owen-mc
Copy link
Contributor

owen-mc commented Dec 15, 2025

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
Copy link
Contributor

owen-mc commented Dec 16, 2025

@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
Copy link
Contributor

owen-mc commented Jan 7, 2026

@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.

@aschackmull aschackmull marked this pull request as ready for review January 9, 2026 09:41
@aschackmull aschackmull requested a review from a team as a code owner January 9, 2026 09:41
Copilot AI review requested due to automatic review settings January 9, 2026 09:41
Copy link
Contributor

Copilot AI left a 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 ParameterizedBarrierGuard module 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/fd.fxlcf.dpdns.org.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.

@aschackmull
Copy link
Contributor Author

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.

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#.

@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Jan 9, 2026
@owen-mc
Copy link
Contributor

owen-mc commented Jan 9, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Go no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants