Swift: Fix 'parameter' -> 'argument' flow into closures.

This commit is contained in:
Mathias Vorreiter Pedersen 2023-10-24 15:23:00 +01:00
Родитель 310ebe47b3
Коммит 1c298e6001
12 изменённых файлов: 90 добавлений и 26 удалений

Просмотреть файл

@ -173,7 +173,7 @@ class ApplyExprCfgNode extends ExprCfgNode {
Callable getStaticTarget() { result = e.getStaticTarget() }
Expr getFunction() { result = e.getFunction() }
CfgNode getFunction() { result.getAst() = e.getFunction() }
}
class CallExprCfgNode extends ApplyExprCfgNode {

Просмотреть файл

@ -293,12 +293,14 @@ private module Cached {
newtype TArgumentPosition =
TThisArgument() or
// we rely on default exprs generated in the caller for ordering
TPositionalArgument(int n) { n = any(Argument arg).getIndex() }
TPositionalArgument(int n) { n = any(Argument arg).getIndex() } or
TClosureSelfArgument()
cached
newtype TParameterPosition =
TThisParameter() or
TPositionalParameter(int n) { n = any(Argument arg).getIndex() }
TPositionalParameter(int n) { n = any(Argument arg).getIndex() } or
TClosureSelfParameter()
}
import Cached
@ -331,6 +333,10 @@ class ThisParameterPosition extends ParameterPosition, TThisParameter {
override string toString() { result = "this" }
}
class ClosureSelfParameter extends ParameterPosition, TClosureSelfParameter {
override string toString() { result = "{ ... }" }
}
/** An argument position. */
class ArgumentPosition extends TArgumentPosition {
/** Gets a textual representation of this position. */
@ -347,6 +353,10 @@ class ThisArgumentPosition extends ArgumentPosition, TThisArgument {
override string toString() { result = "this" }
}
class ClosureSelfArgument extends ArgumentPosition, TClosureSelfArgument {
override string toString() { result = "{ ... }" }
}
/** Holds if arguments at position `apos` match parameters at position `ppos`. */
pragma[inline]
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
@ -354,4 +364,7 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
apos instanceof TThisArgument
or
ppos.(PositionalParameterPosition).getIndex() = apos.(PositionalArgumentPosition).getIndex()
or
ppos instanceof TClosureSelfParameter and
apos instanceof TClosureSelfArgument
}

Просмотреть файл

@ -382,6 +382,12 @@ private class DictionarySubscriptNode extends NodeImpl, TDictionarySubscriptNode
SubscriptExpr getExpr() { result = expr }
}
private class ClosureSelfReferenceNode extends ExprNodeImpl {
override ClosureExpr expr;
ClosureExpr getClosure() { result = expr }
}
private module ParameterNodes {
abstract class ParameterNodeImpl extends NodeImpl {
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
@ -412,6 +418,13 @@ private module ParameterNodes {
override ParamDecl getParameter() { result = param }
}
class ClosureSelfParameterNode extends ParameterNodeImpl, ClosureSelfReferenceNode {
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
c.asSourceCallable() = this.getClosure() and
pos instanceof ClosureSelfParameter
}
}
class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {
SummaryParameterNode() {
FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), _)
@ -626,6 +639,18 @@ private module ArgumentNodes {
pos = TPositionalArgument(0)
}
}
class SelfClosureArgumentNode extends ExprNode, ArgumentNode {
ApplyExprCfgNode apply;
SelfClosureArgumentNode() { n = apply.getFunction() }
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
apply = call.asCall() and
not exists(apply.getStaticTarget()) and
pos instanceof ClosureSelfArgument
}
}
}
import ArgumentNodes
@ -878,8 +903,8 @@ private module CaptureInput implements VariableCapture::InputSig {
source = a.getSource()
)
or
exists(S::PatternBindingDecl pbd, S::NamedPattern np | this = pbd and pbd.getAPattern() = np |
np.getVarDecl() = variable and
exists(S::NamedPattern np | this = np |
variable = np.getVarDecl() and
source = np.getMatchingExpr()
)
// TODO: support multiple variables in LHS of =, in both of above cases.
@ -918,13 +943,23 @@ class CapturedParameter = CaptureInput::CapturedParameter;
module CaptureFlow = VariableCapture::Flow<CaptureInput>;
private CaptureFlow::ClosureNode asClosureNode(Node n) {
result = n.(CaptureNode).getSynthesizedCaptureNode() or
result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
result = n.(CaptureNode).getSynthesizedCaptureNode()
or
result.(CaptureFlow::ExprNode).getExpr() = n.asExpr()
or
result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
n.(PostUpdateNode).getPreUpdateNode().asExpr() or
result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter() or
result.(CaptureFlow::ThisParameterNode).getCallable().getSelfParam() = n.getParameter() or
n.(PostUpdateNode).getPreUpdateNode().asExpr()
or
result.(CaptureFlow::ParameterNode).getParameter() = n.getParameter()
or
result.(CaptureFlow::ThisParameterNode).getCallable() = n.(ClosureSelfParameterNode).getClosure()
or
result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
or
exists(CaptureInput::VariableWrite write |
result.(CaptureFlow::VariableWriteSourceNode).getVariableWrite() = write and
n.asExpr() = write.getSource()
)
}
private predicate captureStoreStep(Node node1, Content::CapturedVariableContent c, Node node2) {

Просмотреть файл

@ -11,7 +11,7 @@ func captureList() {
let y: Int = source("captureList", 123);
{ [x = hello()] () in
sink(x) // $ MISSING: hasValueFlow=hello
sink(y) // $ MISSING: hasValueFlow=captureList
sink(y) // $ hasValueFlow=captureList
}()
}
@ -19,11 +19,11 @@ func setAndCallEscape() {
let x = source("setAndCallEscape", 0)
let escape = {
sink(x) // $ MISSING: hasValueFlow=setAndCallEscape
sink(x) // $ hasValueFlow=setAndCallEscape
return x + 1
}
sink(escape()) // $ MISSING: hasTaintFlow=setAndCallEscape
sink(escape()) // $ hasTaintFlow=setAndCallEscape
}
var escape: (() -> Int)? = nil
@ -31,7 +31,7 @@ var escape: (() -> Int)? = nil
func setEscape() {
let x = source("setEscape", 0)
escape = {
sink(x) // $ MISSING: hasValueFlow=setEscape
sink(x) // $ hasValueFlow=setEscape
return x + 1
}
}
@ -73,7 +73,7 @@ func foo() -> Int {
let f = { y in x += y }
x = source("foo", 41)
let r = { x }
sink(r()) // $ MISSING: hasValueFlow=foo
sink(r()) // $ hasValueFlow=foo
f(1)
return r()
}
@ -138,12 +138,12 @@ func sharedCaptureMultipleWriters() {
func taintCollections(array: inout Array<Int>) {
array[0] = source("array", 0)
sink(array)
sink(array) // $ hasTaintFlow=array
sink(array[0]) // $ hasValueFlow=array
array.withContiguousStorageIfAvailable({
buffer in
sink(array)
sink(array[0]) // $ MISSING: hasValueFlow=array
sink(array) // $ hasTaintFlow=array
sink(array[0]) // $ hasValueFlow=array
})
}

Просмотреть файл

@ -1,2 +1,2 @@
failures
testFailures
failures

Просмотреть файл

@ -47,6 +47,7 @@ edges
| test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
| test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
| test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
| test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
| test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i |
@ -58,6 +59,9 @@ edges
| test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg |
| test.swift:122:31:122:38 | call to source() | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
| test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i |
| test.swift:128:22:131:6 | call to forward(arg:lambda:) | test.swift:132:15:132:15 | clean |
| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg |
| test.swift:128:35:128:42 | call to source() | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
| test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i |
| test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i |
| test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... |
@ -693,6 +697,9 @@ nodes
| test.swift:123:10:123:13 | i | semmle.label | i |
| test.swift:124:16:124:16 | i | semmle.label | i |
| test.swift:126:15:126:15 | z | semmle.label | z |
| test.swift:128:22:131:6 | call to forward(arg:lambda:) | semmle.label | call to forward(arg:lambda:) |
| test.swift:128:35:128:42 | call to source() | semmle.label | call to source() |
| test.swift:132:15:132:15 | clean | semmle.label | clean |
| test.swift:138:19:138:26 | call to source() | semmle.label | call to source() |
| test.swift:142:10:142:13 | i | semmle.label | i |
| test.swift:143:16:143:16 | i | semmle.label | i |
@ -1257,9 +1264,11 @@ nodes
subpaths
| test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
| test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i | test.swift:114:12:114:22 | call to ... |
| test.swift:119:31:119:31 | x | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:119:18:119:44 | call to forward(arg:lambda:) |
| test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
| test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i | test.swift:145:15:145:31 | call to ... |
| test.swift:170:9:170:9 | value | test.swift:163:7:163:7 | value | file://:0:0:0:0 | [post] self [x] | test.swift:170:5:170:5 | [post] self [x] |
| test.swift:174:12:174:12 | self [x] | test.swift:163:7:163:7 | self [x] | file://:0:0:0:0 | .x | test.swift:174:12:174:12 | .x |
@ -1338,6 +1347,7 @@ subpaths
| test.swift:105:19:105:19 | x | test.swift:89:15:89:22 | call to source() | test.swift:105:19:105:19 | x | result |
| test.swift:120:15:120:15 | y | test.swift:118:18:118:25 | call to source() | test.swift:120:15:120:15 | y | result |
| test.swift:126:15:126:15 | z | test.swift:122:31:122:38 | call to source() | test.swift:126:15:126:15 | z | result |
| test.swift:132:15:132:15 | clean | test.swift:128:35:128:42 | call to source() | test.swift:132:15:132:15 | clean | result |
| test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | result |
| test.swift:145:15:145:31 | call to ... | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... | result |
| test.swift:151:15:151:28 | call to ... | test.swift:149:16:149:23 | call to source() | test.swift:151:15:151:28 | call to ... | result |

Просмотреть файл

@ -1,2 +1,2 @@
failures
testFailures
failures

Просмотреть файл

@ -1131,12 +1131,18 @@
| test.swift:888:9:888:9 | SSA def(stream) | test.swift:898:24:898:24 | stream |
| test.swift:888:9:888:9 | stream | test.swift:888:9:888:9 | SSA def(stream) |
| test.swift:888:18:896:6 | call to AsyncStream<Element>.init(_:bufferingPolicy:_:) | test.swift:888:9:888:9 | stream |
| test.swift:889:9:889:9 | continuation | test.swift:890:27:895:13 | continuation |
| test.swift:890:27:895:13 | { ... } | test.swift:891:17:891:17 | phi(this) |
| test.swift:891:17:891:17 | $generator | test.swift:891:17:891:17 | &... |
| test.swift:891:17:891:17 | &... | test.swift:891:17:891:17 | $generator |
| test.swift:891:17:891:17 | [post] $generator | test.swift:891:17:891:17 | &... |
| test.swift:891:17:891:17 | phi(this) | test.swift:892:21:892:21 | this |
| test.swift:891:17:891:17 | phi(this) | test.swift:894:17:894:17 | this |
| test.swift:891:26:891:26 | $generator | test.swift:891:26:891:26 | SSA def($generator) |
| test.swift:891:26:891:26 | SSA def($generator) | test.swift:891:17:891:17 | $generator |
| test.swift:891:26:891:30 | call to makeIterator() | test.swift:891:26:891:26 | $generator |
| test.swift:892:21:892:21 | this | test.swift:891:17:891:17 | phi(this) |
| test.swift:892:21:892:21 | this | test.swift:891:17:891:17 | phi(this) |
| test.swift:898:5:898:5 | $i$generator | test.swift:898:5:898:5 | &... |
| test.swift:898:5:898:5 | &... | test.swift:898:5:898:5 | $i$generator |
| test.swift:898:5:898:5 | [post] $i$generator | test.swift:898:5:898:5 | &... |

Просмотреть файл

@ -129,7 +129,7 @@ func forwarder() {
(i: Int) -> Int in
return 0
})
sink(arg: clean)
sink(arg: clean) // $ SPURIOUS: flow=128
}
func lambdaFlows() {

Просмотреть файл

@ -1,2 +1,2 @@
failures
testFailures
failures

Просмотреть файл

@ -103,8 +103,8 @@ func taintThroughMutablePointer() {
ptr4 in
let return5 = myArray5.withUnsafeBytes({
ptr5 in
sink(arg: ptr5)
sink(arg: ptr5[0]) // $ MISSING: tainted=97
sink(arg: ptr5) // $ tainted=97
sink(arg: ptr5[0]) // $ tainted=97
ptr4.copyBytes(from: ptr5)
sink(arg: ptr4)
sink(arg: ptr4[0]) // $ MISSING: tainted=97
@ -146,7 +146,7 @@ func taintCollections(array: inout Array<Int>, contiguousArray: inout Contiguous
buffer in
sink(arg: buffer) // $ tainted=142
sink(arg: buffer[0]) // $ tainted=142
sink(arg: array)
sink(arg: array) // $ tainted=142
sink(arg: array[0]) // $ tainted=142
})

Просмотреть файл

@ -287,7 +287,7 @@ func taintThroughURL() {
let _ = clean.withCString({
ptrClean in
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: nil))
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ MISSING: tainted=210
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
});
sink(arg: URL(fileURLWithFileSystemRepresentation: 0 as! UnsafePointer<Int8>, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
let _ = tainted.withCString({