Skip to content

Unified: Support property source/sink models in LegacyMaD - #22935

Draft
asgerf wants to merge 1 commit into
mainfrom
asgerf-legacy-mad-property-models
Draft

asgerf wants to merge 1 commit into
mainfrom
asgerf-legacy-mad-property-models

Conversation

@asgerf

@asgerf asgerf commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Legacy Swift models include sources and sinks that refer to properties rather than calls (e.g. UITextField.text, Process.arguments, URLSessionConfiguration.tlsMinimumSupportedProtocolVersion). LegacyMaD.qll previously only understood method/constructor models, so these were ignored.

Approach

  • Selector now covers both call-site selectors (MkCallSelector) and property selectors (MkPropertySelector). Property selectors are built from source/sink rows whose name has no argument list. A new matchesPropertyAccess sits alongside matchesCall and matchesCallable.
  • A property access is matched when its base has an inferred type that is the modeled type (or a subclass, if subtypes is true). Accesses that are call callees are excluded.
  • Sources (output "") are the value of the property read.
  • Sinks (input PostUpdate) are the value assigned to the property, plus the post-update node of the property itself (to catch in-place mutation like task.arguments.append(x)). This deviates from legacy Swift, which used the post-update node of the base; unified has no implicit taint reads, so that would not be useful here.
  • Summary models are unchanged and remain call-only.

Builtins

Outside of tests, declarations from dependencies are not extracted, so types like URL or Process would never be resolved by type inference. To compensate, tools/builtins/types.swift now contains a representative declaration (type hierarchy only, no members) for every type mentioned in legacy-swift.model.yml. The mad test has a missingBuiltinType query that guards against model types lacking a builtin, and a new mad-builtins test exercises property models with no stubs at all.

Notes for review

  • Nested types (e.g. Realm.Configuration, URL.AsyncBytes) are declared but not yet resolved by type inference, so property models on them still don't match. These test cases are marked MISSING.
  • Some test files declare their own stubs that now duplicate builtins; results are unaffected.
  • CommandInjection gains 6 previously missing alerts (executableURL writes and arguments?.append); the corresponding TODO in the query is removed.

Property models (e.g. UITextField.text, Process.arguments) are matched
against member accesses using type inference on the base. Selector now
covers both call-site and property selectors.

Also add stubs for all types mentioned in models to the builtins, since
dependency declarations are not extracted outside of tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant