Unified: Port old MaD models from swift by asgerf · Pull Request #22662 · github/codeql

asgerf · 2026-09-23T12:30:06Z

Ports over the MaD models from old Swift and implements an approximate MaD interpreter for them.

This finds approximately 5700 sinks and 100 sources in the current DCA suite.

Proper MaD support is planned for Q2 and this is mainly to get some data to test with, not to build a long-term solution.
Internally these models are referred to as legacy.

Changes to the MaD models

The models themselves were rewritten a bit to better work in a context where we we can only see the call site, without the call target. The old models mentioned named parameters in a certain order, and argument positions were numeric relative to that order, e.g:

foo(orange:apple:);Argument[1] // refers to the 'apple:' argument

We rewrite these so argument positions refer to the named argument directly

foo(orange:apple:);Argument[apple:]

We then also sort the argument labels so it becomes a canonical signature we can join on

foo(apple:orange:);Argument[apple:]

Limitations of the MaD interpreter

The MaD interpreter has two main limitations:

The receiver type is largely ignored, except when detecting constructor calls and method-override parameters.
Parameters with default values are not supported. We can't tell whether a call foo(apple: 123) actually targets foo(apple:orange:) because we don't know if orange: has a default value. I'm not sure how much this matters in practice.
Only sources and sinks are supported; no summaries or barriers.
Properties such UITextField.text are not supported; only sources and sinks derived from a call or callable are found. It would likely be too imprecise to match properties without looking at the base type, since we can't use arity or argument labels to narrow it down.

Also a few (currently) non-limitations because the MaD models don't use these features:

The input/output access path associated with a source/sink must have length 1 (e.g Argument[1] works but not Argument[1].OptionalSome). Longer access paths appear in summaries, but not for source/sink models.
Callbacks are not handled. Because the access paths we deal with have length 1, it's not currently possible to refer to a parameter or return from a callback.

Summary / Telemetry data

The PR also adds a summary query reporting the number of taint sources and sinks. It uses the existing format from Rust and just leaves out unpopulated fields like taint reach. This DCA PR adds the corresponding taint data summary.

The PR also adds meta-queries for sources and sinks.

DCA runs

Latest DCA that has finished
Another DCA run is underway with the last few commits.

Note that the new summary table isn't quite working yet, but the sources and sinks can be seen as meta-query results.

The original models referred to named arguments as positional arguments, based on the declaration order in the target method, and -1 for the receiver. Change this to Argument[name:] for named arguments, and Argument[self] for the receiver.

Sorting the argument labels simplifies matching with calls

We'll want to avoid importing internal files from queries so LegacyMad will have a "public" API though I expect it to be rewritten eventually.

+  key = "Taint sources - active" and value = count(DataFlow::Node n | Models::isSource(n, "remote"))
+  or
+  key = "Taint sources - disabled" and
+  value = count(DataFlow::Node n | Models::isSource(n, any(string s | s != "remote")))
+  or
+  key = "Taint sources - sensitive data" and none()
+  or
+  key = "Taint edges - number of edges" and none()
+  or
+  key = "Taint reach - nodes tainted" and none()
+  or
+  key = "Taint reach - total non-summary nodes" and none()
+  or
+  key = "Taint reach - per million nodes" and none()
+  or
+  key = "Taint sinks - query sinks" and value = count(DataFlow::Node n | Models::isSink(n, _))


+  key = "Taint sources - active" and value = count(DataFlow::Node n | Models::isSource(n, "remote"))
+  or
+  key = "Taint sources - disabled" and
+  value = count(DataFlow::Node n | Models::isSource(n, any(string s | s != "remote")))
+  or
+  key = "Taint sources - sensitive data" and none()


+
+from DataFlow::Node node, string kind
+where Models::isSink(node, kind)
+select node, "Sink of kind '" + kind + "'"


+
+from DataFlow::Node node, string kind
+where Models::isSource(node, kind)
+select node, "Source of kind '" + kind + "'"


Copilot

Copilot review overview

🟡 Changes recommended

Several imported property, qualified-constructor, and mixed-argument models can never match under the current interpreter.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Support property read and write selectors · New
Normalize qualified constructor names for matching · New
Match numeric arguments against all call argument indexes · New

What changed in this PR

Ports legacy Swift MaD models into Unified with approximate source/sink interpretation, diagnostics, telemetry, and supporting AST/data-flow APIs.

Changes:

Imports legacy Swift source, sink, and summary models.
Adds approximate source/sink matching and tests.
Adds diagnostic and summary-statistics queries.

File	Description
`unified/ql/test/library-tests/mad/test.swift`	Adds model test cases.
`unified/ql/test/library-tests/mad/test.qlref`	Configures inline-expectation testing.
`unified/ql/test/library-tests/mad/test.ql`	Queries modeled sources and sinks.
`unified/ql/test/library-tests/mad/test.expected`	Records generated expected results.
`unified/ql/src/summary/SummaryStats.ql`	Reports taint statistics.
`unified/ql/src/diagnostic/TaintSources.ql`	Lists modeled sources.
`unified/ql/src/diagnostic/TaintSinks.ql`	Lists modeled sinks.
`unified/ql/lib/unified.qll`	Exposes legacy models publicly.
`unified/ql/lib/qlpack.yml`	Enables model data extensions.
`unified/ql/lib/ext/legacy-swift.model.yml`	Imports legacy Swift models.
`unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll`	Implements approximate model interpretation.
`unified/ql/lib/codeql/unified/internal/FacadeAst.qll`	Adds inheritance and argument helpers.
`unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll`	Uses the node post-update API.
`unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll`	Moves post-update lookup onto `Node`.
`unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll`	Updates post-update node construction.
`unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll`	Updates post-update graph edges.
`unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll`	Adds taint telemetry metrics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+  }
+}
+
+private module Debug {


Copilot

Copilot review overview

🟡 Changes recommended

Constructor .init calls and mixed named/positional argument models currently fail to match.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Explicit .init calls are not matched as constructors · New

Resolved since last review (3)

Support property read and write selectors
Match numeric arguments against all call argument indexes
Normalize qualified constructor names for matching

Copilot

Copilot review overview

🟡 Changes recommended

Three imported os_log models currently select the wrong arguments.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

os_log taint rewrites target wrong arguments after label canonicalization · New

Resolved since last review (1)

Explicit .init calls are not matched as constructors

Some argument indices were not translated correctly. In some cases it seems the original index was out of bounds and we set it to the intended argument.

hvitved

LGTM as a first version. Only one QL doc comment.

asgerf · 2026-09-24T13:23:03Z

The second DCA run found 5700 sources, up from 3000, so it seems the bugfixes had a significant impact. 🎉

asgerf added 8 commits September 22, 2026 08:57

unified: Import MaD models from old swift

592903e

unified: Use named labels

04ec328

The original models referred to named arguments as positional arguments, based on the declaration order in the target method, and -1 for the receiver. Change this to Argument[name:] for named arguments, and Argument[self] for the receiver.

unified: Sort argument labels

c37b51d

Sorting the argument labels simplifies matching with calls

unified: Add approximate MaD interpreter

ceec6c2

unified: Add a test and "public" API

5f65e19

We'll want to avoid importing internal files from queries so LegacyMad will have a "public" API though I expect it to be rewritten eventually.

unified: Add test for unsupported parameter match

a77beac

unified: Detect override-parameter sources

91d22cf

unified: Add stats

0d74896

github-advanced-security Bot found potential problems Sep 23, 2026

View reviewed changes

unified: Add meta queries

785508f

github-advanced-security Bot found potential problems Sep 23, 2026

View reviewed changes

asgerf added the no-change-note-required This PR does not need a change note label Sep 24, 2026

unified: Expose getPostUpdateNode() to avoid internal import

67a9562

asgerf requested a balanced review from Copilot September 24, 2026 08:30

Copilot started reviewing on behalf of asgerf September 24, 2026 08:30 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll

Comment thread unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll Outdated

Comment thread unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll

asgerf added 3 commits September 24, 2026 10:52

unified: Document lack of support for properties

56d5444

unified: Document bug for qualified type access

af35b8f

unified: Handle qualified type names

d92e90f

asgerf requested a balanced review from Copilot September 24, 2026 09:21

Copilot started reviewing on behalf of asgerf September 24, 2026 09:22 View session

github-advanced-security Bot found potential problems Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll

}

}

private module Debug {

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll Outdated

asgerf force-pushed the unified/mad-approx branch 2 times, most recently from d7a7e52 to 60ff4e7 Compare September 24, 2026 09:46

asgerf requested a balanced review from Copilot September 24, 2026 09:47

Copilot started reviewing on behalf of asgerf September 24, 2026 09:47 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/ext/legacy-swift.model.yml Outdated

asgerf force-pushed the unified/mad-approx branch from f6214c6 to 8685882 Compare September 24, 2026 09:57

unified: Fix mistranslated Argument tokens

df72e5e

Some argument indices were not translated correctly. In some cases it seems the original index was out of bounds and we set it to the intended argument.

asgerf added 3 commits September 24, 2026 12:01

unified: Add tests for explicit '.init' at constructor call

cd0f329

unified: Handle explicit .init at call site

d1b45ec

unified: Remove unused import

3d5a0d4

asgerf force-pushed the unified/mad-approx branch from 8685882 to 3d5a0d4 Compare September 24, 2026 10:03

asgerf changed the title ~~Unified: Rough import of old MaD models from swift~~ Unified: Port old MaD models from swift Sep 24, 2026

asgerf marked this pull request as ready for review September 24, 2026 10:43

asgerf requested a review from a team as a code owner September 24, 2026 10:43

unified: Fix issue with bad magic

2b6c187

hvitved requested changes Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/FacadeAst.qll Outdated

Update unified/ql/lib/codeql/unified/internal/FacadeAst.qll

935956c

hvitved approved these changes Sep 24, 2026

View reviewed changes

asgerf merged commit ccaf910 into github:main Sep 24, 2026
16 checks passed

Navigation Menu

Unified: Port old MaD models from swift - #22662

Unified: Port old MaD models from swift#22662
asgerf merged 19 commits into
github:mainfrom
asgerf:unified/mad-approx

asgerf commented Sep 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

hvitved left a comment

Uh oh!

Uh oh!

asgerf commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

asgerf commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes to the MaD models

Limitations of the MaD interpreter

Summary / Telemetry data

DCA runs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asgerf commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asgerf commented Sep 23, 2026 •

edited

Loading