Visitar URL original
Unified: Data flow through enum constructors and operators by asgerf · Pull Request #22751 · github/codeql · GitHub
Skip to content

Unified: Data flow through enum constructors and operators #22751

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.

Sign up for GitHub

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

Merged
asgerf merged 22 commits into from
Oct 9, 2026
+718 −76
Merged

Unified: Data flow through enum constructors and operators #22751

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
dcdf0a0
unified: Add test with flow through enums
asgerf Oct 5, 2026
7aae4bc
unified: Support flow through enum constructors
asgerf Oct 5, 2026
ce622db
unified: Model postfix "!" as a read step
asgerf Oct 5, 2026
a58747a
unified: Model more unary and cast operators
asgerf Oct 5, 2026
488f131
unified: Add basic flow through arrays
asgerf Oct 5, 2026
a0c937c
unified: Also treat "!" as a taint step and update test output
asgerf Oct 5, 2026
dca1b90
unified: Manually add one-argument version of Data.init(contentsOf:)
asgerf Oct 5, 2026
143aef9
unified: Add path-injection specific step through 'path'
asgerf Oct 5, 2026
7c4a2e8
unified: Dont show ExprPattern in path
asgerf Oct 5, 2026
61fc5fd
unified: Improve join order
asgerf Oct 7, 2026
6ab5435
unified: Record consistency errors
asgerf Oct 8, 2026
677f52e
unified: Add consistency exclusion, but record the miss in a test case
asgerf Oct 8, 2026
c811698
unified: Fix bug in test case
asgerf Oct 8, 2026
c413773
unified: Make the test case type-checkable
asgerf Oct 8, 2026
18f97f7
unified: Mention flattening behavior of 'try?'
asgerf Oct 8, 2026
f1aeecb
unified: Add test case for one-arg Data call
asgerf Oct 8, 2026
9211707
unified: Update test output after rebasing
asgerf Oct 8, 2026
303385e
unified: Fix typo
asgerf Oct 8, 2026
cfc8be4
unified: Remove empty DataFlowConsistency.expected
asgerf Oct 9, 2026
d04059b
unified: Factor out EnumCaseClass
asgerf Oct 9, 2026
f4d4777
unified: Use EnumCaseConstructor in TypeInference
asgerf Oct 9, 2026
e103940
unified: Elaborate QLDoc for enum cases
asgerf Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
unified: Also treat "!" as a taint step and update test output 
Note that one of the annotations were moved because the location we report is different
  • Loading branch information
@asgerf
asgerf committed Oct 8, 2026
commit a0c937c332b271daa042f91651d2a78fcb565322
2 changes: 1 addition & 1 deletion unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
exists(UnaryExpr expr |
expr.getOperator().(PostfixOperator).getValue() = "!" and
node1.isResultValue(expr.getOperand()) and
step.readName("some.0") and
(step.readName("some.0") or step.taint()) and

@hvitved hvitved Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Once we lift all read steps to store steps (which I think we should, possibly except a shortlist of undesired contents) then the or step.tain() part should not be needed.

@asgerf asgerf Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely agree, the read->taint translation shouldn't be opt-in on a case-by-case basis as is indicated here. This was just to make a minimal change that didn't break the path-injection test case (where an Optional<URL> is a source and we thus depend on this behaviour).

node2.isResultValue(expr)
or
expr.getOperator().(PrefixOperator).getValue() = ["try", "try!", "await"] and
Expand Down
13 changes: 13 additions & 0 deletions unified/ql/test/query-tests/security/CWE-022/PathInjection/PathInjectionTest.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
| testPathInjection.swift:421:26:421:34 | remoteUrl | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:421:26:421:34 | remoteUrl | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:422:30:422:41 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:422:30:422:41 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:424:59:424:70 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:424:59:424:70 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:425:46:425:76 | TypeCastExpr | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:425:46:425:76 | TypeCastExpr | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:426:42:426:72 | TypeCastExpr | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:426:42:426:72 | TypeCastExpr | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:436:25:436:33 | remoteUrl | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:436:25:436:33 | remoteUrl | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:437:26:437:37 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:437:26:437:37 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
| testPathInjection.swift:441:28:441:39 | remoteString | testPathInjection.swift:340:24:340:78 | String(...) | testPathInjection.swift:441:28:441:39 | remoteString | This path depends on a $@. | testPathInjection.swift:340:24:340:78 | String(...) | user-provided value |
Expand Down Expand Up @@ -125,6 +127,9 @@ edges
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:420:43:420:54 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:422:30:422:41 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:424:59:424:70 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:425:46:425:57 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:426:42:426:53 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:436:25:436:33 | remoteUrl | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:437:26:437:37 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:441:28:441:39 | remoteString | provenance | |
| testPathInjection.swift:340:9:340:20 | remoteString | testPathInjection.swift:443:32:443:43 | remoteString | provenance | |
Expand Down Expand Up @@ -162,6 +167,10 @@ edges
| testPathInjection.swift:342:9:342:19 | remoteNsUrl | testPathInjection.swift:410:26:410:36 | remoteNsUrl | provenance | |
| testPathInjection.swift:342:9:342:19 | remoteNsUrl | testPathInjection.swift:412:52:412:62 | remoteNsUrl | provenance | |
| testPathInjection.swift:342:37:342:48 | remoteString | testPathInjection.swift:342:9:342:19 | remoteNsUrl | provenance | |
| testPathInjection.swift:425:46:425:57 | remoteString | testPathInjection.swift:425:46:425:76 | TypeCastExpr | provenance | |
| testPathInjection.swift:426:42:426:53 | remoteString | testPathInjection.swift:426:42:426:72 | TypeCastExpr | provenance | |
| testPathInjection.swift:477:9:477:18 | remoteData | testPathInjection.swift:484:24:484:30 | buffer2 | provenance | |
| testPathInjection.swift:477:22:477:87 | Data(...) | testPathInjection.swift:477:9:477:18 | remoteData | provenance | |
| testPathInjection.swift:504:9:504:20 | remoteString | testPathInjection.swift:510:37:510:48 | remoteString | provenance | |
| testPathInjection.swift:504:9:504:20 | remoteString | testPathInjection.swift:512:33:512:44 | remoteString | provenance | |
| testPathInjection.swift:504:24:504:78 | String(...) | testPathInjection.swift:504:9:504:20 | remoteString | provenance | |
Expand Down Expand Up @@ -251,6 +260,10 @@ nodes
| testPathInjection.swift:421:26:421:34 | remoteUrl | semmle.label | remoteUrl |
| testPathInjection.swift:422:30:422:41 | remoteString | semmle.label | remoteString |
| testPathInjection.swift:424:59:424:70 | remoteString | semmle.label | remoteString |
| testPathInjection.swift:425:46:425:57 | remoteString | semmle.label | remoteString |
| testPathInjection.swift:425:46:425:76 | TypeCastExpr | semmle.label | TypeCastExpr |
| testPathInjection.swift:426:42:426:53 | remoteString | semmle.label | remoteString |
| testPathInjection.swift:426:42:426:72 | TypeCastExpr | semmle.label | TypeCastExpr |
| testPathInjection.swift:436:25:436:33 | remoteUrl | semmle.label | remoteUrl |
| testPathInjection.swift:437:26:437:37 | remoteString | semmle.label | remoteString |
| testPathInjection.swift:441:28:441:39 | remoteString | semmle.label | remoteString |
Expand Down
6 changes: 3 additions & 3 deletions unified/ql/test/query-tests/security/CWE-022/PathInjection/testPathInjection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,10 @@ func test(buffer1: UnsafeMutablePointer<UInt8>, buffer2: UnsafeMutablePointer<UI
NSString().write(toFile: remoteString, atomically: true, encoding: 0) // $ Alert

let _ = NSKeyedUnarchiver().unarchiveObject(withFile: remoteString) // $ Alert
let _ = ArchiveByteStream.fileStream(fd: remoteString as! FileDescriptor, automaticClose: true) // $ MISSING: Alert
ArchiveByteStream.withFileStream(fd: remoteString as! FileDescriptor, automaticClose: true) {
let _ = ArchiveByteStream.fileStream(fd: remoteString as! FileDescriptor, automaticClose: true) // $ Alert
ArchiveByteStream.withFileStream(fd: remoteString as! FileDescriptor, automaticClose: true) { // $ Alert
_ in
} // $ MISSING: Alert
}
let _ = ArchiveByteStream.fileStream(
path: FilePath(stringLiteral: remoteString), mode: .readOnly, options: .append,
permissions: .ownerRead) // $ MISSING: Alert
Expand Down