Skip to content

Commit 70c2ef5

Browse files
rdmarsh2rdmarsh2
committed
Swift: collection/tuple content for dictionary flow
1 parent 36bdadf commit 70c2ef5

File tree

5 files changed

+193
-0
lines changed

5 files changed

+193
-0
lines changed

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.swift.dataflow.FlowSummary as FlowSummary
99
private import codeql.swift.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
1010
private import codeql.swift.frameworks.StandardLibrary.PointerTypes
1111
private import codeql.swift.frameworks.StandardLibrary.Array
12+
private import codeql.swift.frameworks.StandardLibrary.Dictionary
1213

1314
/** Gets the callable in which this node occurs. */
1415
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.(NodeImpl).getEnclosingCallable() }
@@ -114,6 +115,9 @@ private module Cached {
114115
any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr(),
115116
any(SubscriptExpr se).getBase()
116117
])
118+
} or
119+
TDictionarySubscriptNode(SubscriptExpr e) {
120+
e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
117121
}
118122

119123
private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
@@ -296,6 +300,29 @@ import Cached
296300
/** Holds if `n` should be hidden from path explanations. */
297301
predicate nodeIsHidden(Node n) { n instanceof FlowSummaryNode }
298302

303+
private class DictionarySubscriptNode extends NodeImpl, TDictionarySubscriptNode {
304+
SubscriptExpr expr;
305+
DictionarySubscriptNode() {
306+
this = TDictionarySubscriptNode(expr)
307+
}
308+
309+
override DataFlowCallable getEnclosingCallable() {
310+
result.asSourceCallable() = expr.getEnclosingCallable()
311+
}
312+
313+
override string toStringImpl() {
314+
result = "DictionarySubscriptNode"
315+
}
316+
317+
override Location getLocationImpl() {
318+
result = expr.getLocation()
319+
}
320+
321+
SubscriptExpr getExpr() {
322+
result = expr
323+
}
324+
}
325+
299326
private module ParameterNodes {
300327
abstract class ParameterNodeImpl extends NodeImpl {
301328
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
@@ -727,6 +754,30 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
727754
c.isSingleton(any(Content::ArrayContent ac))
728755
)
729756
or
757+
// read of a dictionary value via subscript operator, with intermediate step
758+
exists(AssignExpr assign, SubscriptExpr subscript |
759+
subscript = assign.getDest() and
760+
(
761+
subscript.getArgument(0).getExpr() = node1.asExpr() and
762+
node2.(DictionarySubscriptNode).getExpr() = subscript and
763+
c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = 1))
764+
or
765+
assign.getSource() = node1.asExpr() and
766+
node2.(DictionarySubscriptNode).getExpr() = subscript and
767+
c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = 1))
768+
or
769+
node1.(DictionarySubscriptNode) = node1 and
770+
node2.asExpr() = subscript and
771+
c.isSingleton(any(Content::CollectionContent cc))
772+
)
773+
)
774+
or
775+
exists(DictionaryExpr dict |
776+
node1.asExpr() = dict.getAnElement() and
777+
node2.asExpr() = dict and
778+
c.isSingleton(any(Content::CollectionContent cc))
779+
)
780+
or
730781
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
731782
node2.(FlowSummaryNode).getSummaryNode())
732783
}
@@ -807,6 +858,17 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
807858
c.isSingleton(any(Content::ArrayContent ac))
808859
)
809860
or
861+
// read of a dictionary value via subscript operator
862+
exists(SubscriptExpr subscript |
863+
subscript.getBase() = node1.asExpr() and
864+
node2.(DictionarySubscriptNode).getExpr() = subscript and
865+
c.isSingleton(any(Content::CollectionContent cc))
866+
or
867+
subscript = node2.asExpr() and
868+
node1.(DictionarySubscriptNode).getExpr() = subscript and
869+
c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = 1))
870+
)
871+
or
810872
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
811873
node2.(FlowSummaryNode).getSummaryNode())
812874
}

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Dictionary.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import swift
2+
private import codeql.swift.dataflow.ExternalFlow
3+
4+
/**
5+
* An instance of the `Dictionary` type.
6+
*/
7+
class CanonicalDictionaryType extends BoundGenericType {
8+
CanonicalDictionaryType() { this.getName().matches("Dictionary<%") }
9+
}

‎swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,20 @@ edges
404404
| test.swift:756:15:756:19 | .v2 [some:0] | test.swift:756:15:756:21 | ...! |
405405
| test.swift:757:15:757:15 | mo1 [v3] | test.swift:732:9:732:9 | self [v3] |
406406
| test.swift:757:15:757:15 | mo1 [v3] | test.swift:757:15:757:19 | .v3 |
407+
| test.swift:767:5:767:5 | [post] dict1 [Array element] | test.swift:769:15:769:15 | dict1 [Array element] |
408+
| test.swift:767:16:767:23 | call to source() | test.swift:767:5:767:5 | [post] dict1 [Array element] |
409+
| test.swift:769:15:769:15 | dict1 [Array element] | test.swift:769:15:769:22 | ...[...] |
410+
| test.swift:779:17:779:29 | [...] [Collection element, Tuple element at index 1] | test.swift:780:15:780:15 | dict3 [Collection element, Tuple element at index 1] |
411+
| test.swift:779:18:779:28 | (...) [Tuple element at index 1] | test.swift:779:17:779:29 | [...] [Collection element, Tuple element at index 1] |
412+
| test.swift:779:21:779:28 | call to source() | test.swift:779:18:779:28 | (...) [Tuple element at index 1] |
413+
| test.swift:780:15:780:15 | dict3 [Collection element, Tuple element at index 1] | test.swift:780:15:780:22 | DictionarySubscriptNode [Tuple element at index 1] |
414+
| test.swift:780:15:780:22 | DictionarySubscriptNode [Tuple element at index 1] | test.swift:780:15:780:22 | ...[...] |
415+
| test.swift:789:17:789:28 | [...] [Collection element, Tuple element at index 1] | test.swift:793:15:793:15 | dict4 [Collection element, Tuple element at index 1] |
416+
| test.swift:789:18:789:27 | (...) [Tuple element at index 1] | test.swift:789:17:789:28 | [...] [Collection element, Tuple element at index 1] |
417+
| test.swift:789:20:789:27 | call to source() | test.swift:789:18:789:27 | (...) [Tuple element at index 1] |
418+
| test.swift:793:15:793:15 | dict4 [Collection element, Tuple element at index 1] | test.swift:793:15:793:35 | call to randomElement() [some:0, Tuple element at index 1] |
419+
| test.swift:793:15:793:35 | call to randomElement() [some:0, Tuple element at index 1] | test.swift:793:15:793:36 | ...! [Tuple element at index 1] |
420+
| test.swift:793:15:793:36 | ...! [Tuple element at index 1] | test.swift:793:15:793:38 | .1 |
407421
nodes
408422
| file://:0:0:0:0 | .a [x] | semmle.label | .a [x] |
409423
| file://:0:0:0:0 | .str | semmle.label | .str |
@@ -849,6 +863,23 @@ nodes
849863
| test.swift:756:15:756:21 | ...! | semmle.label | ...! |
850864
| test.swift:757:15:757:15 | mo1 [v3] | semmle.label | mo1 [v3] |
851865
| test.swift:757:15:757:19 | .v3 | semmle.label | .v3 |
866+
| test.swift:767:5:767:5 | [post] dict1 [Array element] | semmle.label | [post] dict1 [Array element] |
867+
| test.swift:767:16:767:23 | call to source() | semmle.label | call to source() |
868+
| test.swift:769:15:769:15 | dict1 [Array element] | semmle.label | dict1 [Array element] |
869+
| test.swift:769:15:769:22 | ...[...] | semmle.label | ...[...] |
870+
| test.swift:779:17:779:29 | [...] [Collection element, Tuple element at index 1] | semmle.label | [...] [Collection element, Tuple element at index 1] |
871+
| test.swift:779:18:779:28 | (...) [Tuple element at index 1] | semmle.label | (...) [Tuple element at index 1] |
872+
| test.swift:779:21:779:28 | call to source() | semmle.label | call to source() |
873+
| test.swift:780:15:780:15 | dict3 [Collection element, Tuple element at index 1] | semmle.label | dict3 [Collection element, Tuple element at index 1] |
874+
| test.swift:780:15:780:22 | ...[...] | semmle.label | ...[...] |
875+
| test.swift:780:15:780:22 | DictionarySubscriptNode [Tuple element at index 1] | semmle.label | DictionarySubscriptNode [Tuple element at index 1] |
876+
| test.swift:789:17:789:28 | [...] [Collection element, Tuple element at index 1] | semmle.label | [...] [Collection element, Tuple element at index 1] |
877+
| test.swift:789:18:789:27 | (...) [Tuple element at index 1] | semmle.label | (...) [Tuple element at index 1] |
878+
| test.swift:789:20:789:27 | call to source() | semmle.label | call to source() |
879+
| test.swift:793:15:793:15 | dict4 [Collection element, Tuple element at index 1] | semmle.label | dict4 [Collection element, Tuple element at index 1] |
880+
| test.swift:793:15:793:35 | call to randomElement() [some:0, Tuple element at index 1] | semmle.label | call to randomElement() [some:0, Tuple element at index 1] |
881+
| test.swift:793:15:793:36 | ...! [Tuple element at index 1] | semmle.label | ...! [Tuple element at index 1] |
882+
| test.swift:793:15:793:38 | .1 | semmle.label | .1 |
852883
subpaths
853884
| 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 |
854885
| 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 ... |
@@ -996,3 +1027,6 @@ subpaths
9961027
| test.swift:754:15:754:15 | v3 | test.swift:744:10:744:17 | call to source() | test.swift:754:15:754:15 | v3 | result |
9971028
| test.swift:756:15:756:21 | ...! | test.swift:746:14:746:21 | call to source() | test.swift:756:15:756:21 | ...! | result |
9981029
| test.swift:757:15:757:19 | .v3 | test.swift:747:14:747:21 | call to source() | test.swift:757:15:757:19 | .v3 | result |
1030+
| test.swift:769:15:769:22 | ...[...] | test.swift:767:16:767:23 | call to source() | test.swift:769:15:769:22 | ...[...] | result |
1031+
| test.swift:780:15:780:22 | ...[...] | test.swift:779:21:779:28 | call to source() | test.swift:780:15:780:22 | ...[...] | result |
1032+
| test.swift:793:15:793:38 | .1 | test.swift:789:20:789:27 | call to source() | test.swift:793:15:793:38 | .1 | result |

‎swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,3 +913,56 @@
913913
| test.swift:759:15:759:15 | mo2 | test.swift:760:15:760:15 | mo2 |
914914
| test.swift:759:15:759:20 | .v2 | test.swift:759:15:759:22 | ...! |
915915
| test.swift:760:15:760:15 | mo2 | test.swift:760:15:760:18 | ...! |
916+
| test.swift:764:9:764:9 | SSA def(dict1) | test.swift:765:15:765:15 | dict1 |
917+
| test.swift:764:9:764:9 | dict1 | test.swift:764:9:764:9 | SSA def(dict1) |
918+
| test.swift:764:17:764:31 | [...] | test.swift:764:9:764:9 | dict1 |
919+
| test.swift:765:15:765:15 | &... | test.swift:767:5:767:5 | dict1 |
920+
| test.swift:765:15:765:15 | [post] dict1 | test.swift:765:15:765:15 | &... |
921+
| test.swift:765:15:765:15 | dict1 | test.swift:765:15:765:15 | &... |
922+
| test.swift:767:5:767:5 | &... | test.swift:769:15:769:15 | dict1 |
923+
| test.swift:767:5:767:5 | [post] dict1 | test.swift:767:5:767:5 | &... |
924+
| test.swift:767:5:767:5 | dict1 | test.swift:767:5:767:5 | &... |
925+
| test.swift:769:15:769:15 | [post] dict1 | test.swift:769:15:769:15 | &... |
926+
| test.swift:769:15:769:15 | dict1 | test.swift:769:15:769:15 | &... |
927+
| test.swift:771:9:771:9 | SSA def(dict2) | test.swift:772:15:772:15 | dict2 |
928+
| test.swift:771:9:771:9 | dict2 | test.swift:771:9:771:9 | SSA def(dict2) |
929+
| test.swift:771:17:771:29 | [...] | test.swift:771:9:771:9 | dict2 |
930+
| test.swift:772:15:772:15 | &... | test.swift:774:25:774:25 | dict2 |
931+
| test.swift:772:15:772:15 | [post] dict2 | test.swift:772:15:772:15 | &... |
932+
| test.swift:772:15:772:15 | dict2 | test.swift:772:15:772:15 | &... |
933+
| test.swift:774:10:774:10 | SSA def(key) | test.swift:775:19:775:19 | key |
934+
| test.swift:774:10:774:10 | key | test.swift:774:10:774:10 | SSA def(key) |
935+
| test.swift:774:15:774:15 | SSA def(value) | test.swift:776:19:776:19 | value |
936+
| test.swift:774:15:774:15 | value | test.swift:774:15:774:15 | SSA def(value) |
937+
| test.swift:779:9:779:9 | SSA def(dict3) | test.swift:780:15:780:15 | dict3 |
938+
| test.swift:779:9:779:9 | dict3 | test.swift:779:9:779:9 | SSA def(dict3) |
939+
| test.swift:779:17:779:29 | [...] | test.swift:779:9:779:9 | dict3 |
940+
| test.swift:780:15:780:15 | &... | test.swift:782:5:782:5 | dict3 |
941+
| test.swift:780:15:780:15 | [post] dict3 | test.swift:780:15:780:15 | &... |
942+
| test.swift:780:15:780:15 | dict3 | test.swift:780:15:780:15 | &... |
943+
| test.swift:782:5:782:5 | &... | test.swift:784:25:784:25 | dict3 |
944+
| test.swift:782:5:782:5 | [post] dict3 | test.swift:782:5:782:5 | &... |
945+
| test.swift:782:5:782:5 | dict3 | test.swift:782:5:782:5 | &... |
946+
| test.swift:784:10:784:10 | SSA def(key) | test.swift:785:19:785:19 | key |
947+
| test.swift:784:10:784:10 | key | test.swift:784:10:784:10 | SSA def(key) |
948+
| test.swift:784:15:784:15 | SSA def(value) | test.swift:786:19:786:19 | value |
949+
| test.swift:784:15:784:15 | value | test.swift:784:15:784:15 | SSA def(value) |
950+
| test.swift:789:9:789:9 | SSA def(dict4) | test.swift:790:15:790:15 | dict4 |
951+
| test.swift:789:9:789:9 | dict4 | test.swift:789:9:789:9 | SSA def(dict4) |
952+
| test.swift:789:17:789:28 | [...] | test.swift:789:9:789:9 | dict4 |
953+
| test.swift:790:15:790:15 | &... | test.swift:791:15:791:15 | dict4 |
954+
| test.swift:790:15:790:15 | [post] dict4 | test.swift:790:15:790:15 | &... |
955+
| test.swift:790:15:790:15 | dict4 | test.swift:790:15:790:15 | &... |
956+
| test.swift:790:15:790:52 | call to updateValue(_:forKey:) | test.swift:790:15:790:53 | ...! |
957+
| test.swift:791:15:791:15 | &... | test.swift:792:15:792:15 | dict4 |
958+
| test.swift:791:15:791:15 | [post] dict4 | test.swift:791:15:791:15 | &... |
959+
| test.swift:791:15:791:15 | dict4 | test.swift:791:15:791:15 | &... |
960+
| test.swift:791:15:791:52 | call to updateValue(_:forKey:) | test.swift:791:15:791:53 | ...! |
961+
| test.swift:792:15:792:15 | [post] dict4 | test.swift:793:15:793:15 | dict4 |
962+
| test.swift:792:15:792:15 | dict4 | test.swift:793:15:793:15 | dict4 |
963+
| test.swift:792:15:792:35 | call to randomElement() | test.swift:792:15:792:36 | ...! |
964+
| test.swift:793:15:793:15 | [post] dict4 | test.swift:794:15:794:15 | dict4 |
965+
| test.swift:793:15:793:15 | dict4 | test.swift:794:15:794:15 | dict4 |
966+
| test.swift:793:15:793:35 | call to randomElement() | test.swift:793:15:793:36 | ...! |
967+
| test.swift:794:15:794:15 | [post] dict4 | test.swift:795:15:795:15 | dict4 |
968+
| test.swift:794:15:794:15 | dict4 | test.swift:795:15:795:15 | dict4 |

‎swift/ql/test/library-tests/dataflow/dataflow/test.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,3 +759,38 @@ func testWriteOptional() {
759759
sink(arg: mo2!.v2!) // $ MISSING:flow=749
760760
sink(arg: mo2!.v3) // $ MISSING:flow=750
761761
}
762+
763+
func testDictionary() {
764+
var dict1 = [1:2, 3:4, 5:6]
765+
sink(arg: dict1[1])
766+
767+
dict1[1] = source()
768+
769+
sink(arg: dict1[1]) // $ flow=767
770+
771+
var dict2 = [source(): 1]
772+
sink(arg: dict2[1])
773+
774+
for (key, value) in dict2 {
775+
sink(arg: key) // $ MISSING: flow=771
776+
sink(arg: value)
777+
}
778+
779+
var dict3 = [1: source()]
780+
sink(arg: dict3[1]) // $ flow=779
781+
782+
dict3[source()] = 2
783+
784+
for (key, value) in dict3 {
785+
sink(arg: key) // $ MISSING: flow=782
786+
sink(arg: value) // $ MISSING: flow=779
787+
}
788+
789+
var dict4 = [1:source()]
790+
sink(arg: dict4.updateValue(1, forKey: source())!)
791+
sink(arg: dict4.updateValue(source(), forKey: 2)!)
792+
sink(arg: dict4.randomElement()!.0) // $ MISSING: flow=791
793+
sink(arg: dict4.randomElement()!.1) // $ flow=789 MISSING: flow=790
794+
sink(arg: dict4.keys.randomElement()) // $ MISSING: flow=791
795+
sink(arg: dict4.values.randomElement()) // $ MISSING: flow=789 flow=790
796+
}

0 commit comments

Comments
 (0)