Skip to content

Commit d888510

Browse files
geoffw0geoffw0
committed
Swift: Fix incorrect taint to String fields.
1 parent d25de8c commit d888510

File tree

4 files changed

+16
-23
lines changed

4 files changed

+16
-23
lines changed

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,20 @@ private class StringSource extends SourceModelCsv {
2626
private class StringFieldsInheritTaint extends TaintInheritingContent,
2727
DataFlow::Content::FieldContent {
2828
StringFieldsInheritTaint() {
29-
this.getField().getEnclosingDecl().(NominalTypeDecl).getFullName() =
30-
["String", "StringProtocol"] or
31-
this.getField().getEnclosingDecl().(ExtensionDecl).getExtendedTypeDecl().getFullName() =
32-
["String", "StringProtocol"]
29+
exists(FieldDecl f | this.getField() = f |
30+
(
31+
f.getEnclosingDecl().(NominalTypeDecl).getName() = ["String", "StringProtocol"] or
32+
f.getEnclosingDecl().(ExtensionDecl).getExtendedTypeDecl().getName() =
33+
["String", "StringProtocol"]
34+
) and
35+
f.getName() =
36+
[
37+
"first", "last", "unicodeScalars", "utf8", "utf16", "lazy", "utf8CString", "description",
38+
"debugDescription", "dataValue", "identifierValue", "capitalized", "localizedCapitalized",
39+
"localizedLowercase", "localizedUppercase", "decomposedStringWithCanonicalMapping",
40+
"decomposedStringWithCompatibilityMapping", "precomposedStringWithCanonicalMapping",
41+
"precomposedStringWithCompatibilityMapping", "removingPercentEncoding"
42+
]
43+
)
3344
}
3445
}

‎swift/ql/test/library-tests/dataflow/taint/LocalTaint.expected

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,15 +1122,13 @@
11221122
| string.swift:204:3:204:3 | [post] &... | string.swift:204:38:204:38 | str5 |
11231123
| string.swift:204:3:204:3 | str5 | string.swift:204:3:204:3 | &... |
11241124
| string.swift:204:38:204:38 | [post] str5 | string.swift:205:13:205:13 | str5 |
1125-
| string.swift:204:38:204:38 | str5 | string.swift:204:38:204:43 | .startIndex |
11261125
| string.swift:204:38:204:38 | str5 | string.swift:205:13:205:13 | str5 |
11271126
| string.swift:205:13:205:13 | [post] str5 | string.swift:206:3:206:3 | str5 |
11281127
| string.swift:205:13:205:13 | str5 | string.swift:206:3:206:3 | str5 |
11291128
| string.swift:206:3:206:3 | &... | string.swift:206:42:206:42 | str5 |
11301129
| string.swift:206:3:206:3 | [post] &... | string.swift:206:42:206:42 | str5 |
11311130
| string.swift:206:3:206:3 | str5 | string.swift:206:3:206:3 | &... |
11321131
| string.swift:206:42:206:42 | [post] str5 | string.swift:207:13:207:13 | str5 |
1133-
| string.swift:206:42:206:42 | str5 | string.swift:206:42:206:47 | .startIndex |
11341132
| string.swift:206:42:206:42 | str5 | string.swift:207:13:207:13 | str5 |
11351133
| string.swift:211:7:211:7 | SSA def(clean) | string.swift:215:20:215:20 | clean |
11361134
| string.swift:211:15:211:15 | | string.swift:211:7:211:7 | SSA def(clean) |
@@ -1166,7 +1164,6 @@
11661164
| string.swift:232:13:232:13 | [post] tainted | string.swift:232:37:232:37 | tainted |
11671165
| string.swift:232:13:232:13 | tainted | string.swift:232:37:232:37 | tainted |
11681166
| string.swift:232:37:232:37 | [post] tainted | string.swift:234:13:234:13 | tainted |
1169-
| string.swift:232:37:232:37 | tainted | string.swift:232:37:232:45 | .startIndex |
11701167
| string.swift:232:37:232:37 | tainted | string.swift:234:13:234:13 | tainted |
11711168
| string.swift:234:13:234:13 | [post] tainted | string.swift:235:13:235:13 | tainted |
11721169
| string.swift:234:13:234:13 | tainted | string.swift:235:13:235:13 | tainted |
@@ -1311,7 +1308,6 @@
13111308
| string.swift:302:13:302:13 | [post] &... | string.swift:302:29:302:29 | str1 |
13121309
| string.swift:302:13:302:13 | str1 | string.swift:302:13:302:13 | &... |
13131310
| string.swift:302:29:302:29 | [post] str1 | string.swift:303:13:303:13 | str1 |
1314-
| string.swift:302:29:302:29 | str1 | string.swift:302:29:302:34 | .startIndex |
13151311
| string.swift:302:29:302:29 | str1 | string.swift:303:13:303:13 | str1 |
13161312
| string.swift:305:7:305:7 | SSA def(str2) | string.swift:306:13:306:13 | str2 |
13171313
| string.swift:305:14:305:22 | call to source2() | string.swift:305:7:305:7 | SSA def(str2) |
@@ -1357,12 +1353,10 @@
13571353
| string.swift:328:3:328:3 | [post] &... | string.swift:328:23:328:23 | str5 |
13581354
| string.swift:328:3:328:3 | str5 | string.swift:328:3:328:3 | &... |
13591355
| string.swift:328:23:328:23 | [post] str5 | string.swift:328:43:328:43 | str5 |
1360-
| string.swift:328:23:328:23 | str5 | string.swift:328:23:328:28 | .startIndex |
13611356
| string.swift:328:23:328:23 | str5 | string.swift:328:43:328:43 | str5 |
13621357
| string.swift:328:43:328:43 | [post] str5 | string.swift:328:54:328:54 | str5 |
13631358
| string.swift:328:43:328:43 | str5 | string.swift:328:54:328:54 | str5 |
13641359
| string.swift:328:54:328:54 | [post] str5 | string.swift:329:13:329:13 | str5 |
1365-
| string.swift:328:54:328:54 | str5 | string.swift:328:54:328:59 | .startIndex |
13661360
| string.swift:328:54:328:54 | str5 | string.swift:329:13:329:13 | str5 |
13671361
| string.swift:331:7:331:7 | SSA def(str6) | string.swift:332:13:332:13 | str6 |
13681362
| string.swift:331:14:331:22 | call to source2() | string.swift:331:7:331:7 | SSA def(str6) |
@@ -1611,10 +1605,8 @@
16111605
| string.swift:544:14:544:14 | tainted | string.swift:544:22:544:22 | tainted |
16121606
| string.swift:544:14:544:61 | ...[...] | string.swift:544:7:544:7 | SSA def(sub1) |
16131607
| string.swift:544:22:544:22 | [post] tainted | string.swift:544:45:544:45 | tainted |
1614-
| string.swift:544:22:544:22 | tainted | string.swift:544:22:544:30 | .startIndex |
16151608
| string.swift:544:22:544:22 | tainted | string.swift:544:45:544:45 | tainted |
16161609
| string.swift:544:45:544:45 | [post] tainted | string.swift:548:14:548:14 | tainted |
1617-
| string.swift:544:45:544:45 | tainted | string.swift:544:45:544:53 | .endIndex |
16181610
| string.swift:544:45:544:45 | tainted | string.swift:548:14:548:14 | tainted |
16191611
| string.swift:545:13:545:13 | [post] sub1 | string.swift:546:20:546:20 | sub1 |
16201612
| string.swift:545:13:545:13 | sub1 | string.swift:546:20:546:20 | sub1 |
@@ -1628,15 +1620,13 @@
16281620
| string.swift:552:14:552:14 | tainted | string.swift:552:38:552:38 | tainted |
16291621
| string.swift:552:14:552:54 | call to prefix(through:) | string.swift:552:7:552:7 | SSA def(sub3) |
16301622
| string.swift:552:38:552:38 | [post] tainted | string.swift:556:14:556:14 | tainted |
1631-
| string.swift:552:38:552:38 | tainted | string.swift:552:38:552:46 | .endIndex |
16321623
| string.swift:552:38:552:38 | tainted | string.swift:556:14:556:14 | tainted |
16331624
| string.swift:553:13:553:13 | sub3 | string.swift:554:20:554:20 | sub3 |
16341625
| string.swift:556:7:556:7 | SSA def(sub4) | string.swift:557:13:557:13 | sub4 |
16351626
| string.swift:556:14:556:14 | [post] tainted | string.swift:556:35:556:35 | tainted |
16361627
| string.swift:556:14:556:14 | tainted | string.swift:556:35:556:35 | tainted |
16371628
| string.swift:556:14:556:51 | call to prefix(upTo:) | string.swift:556:7:556:7 | SSA def(sub4) |
16381629
| string.swift:556:35:556:35 | [post] tainted | string.swift:560:14:560:14 | tainted |
1639-
| string.swift:556:35:556:35 | tainted | string.swift:556:35:556:43 | .endIndex |
16401630
| string.swift:556:35:556:35 | tainted | string.swift:560:14:560:14 | tainted |
16411631
| string.swift:557:13:557:13 | sub4 | string.swift:558:20:558:20 | sub4 |
16421632
| string.swift:560:7:560:7 | SSA def(sub5) | string.swift:561:13:561:13 | sub5 |
@@ -1648,7 +1638,6 @@
16481638
| string.swift:564:14:564:14 | [post] tainted | string.swift:564:35:564:35 | tainted |
16491639
| string.swift:564:14:564:14 | tainted | string.swift:564:35:564:35 | tainted |
16501640
| string.swift:564:14:564:53 | call to suffix(from:) | string.swift:564:7:564:7 | SSA def(sub6) |
1651-
| string.swift:564:35:564:35 | tainted | string.swift:564:35:564:43 | .startIndex |
16521641
| string.swift:565:13:565:13 | sub6 | string.swift:566:20:566:20 | sub6 |
16531642
| string.swift:570:7:570:7 | SSA def(clean) | string.swift:573:13:573:13 | clean |
16541643
| string.swift:570:15:570:26 | call to FilePath.init(_:) | string.swift:570:7:570:7 | SSA def(clean) |
@@ -1717,9 +1706,6 @@
17171706
| string.swift:629:13:629:26 | call to Self.init(_:) | string.swift:629:13:629:27 | ...! |
17181707
| string.swift:633:7:633:7 | SSA def(tainted) | string.swift:637:13:637:13 | tainted |
17191708
| string.swift:633:17:633:25 | call to source2() | string.swift:633:7:633:7 | SSA def(tainted) |
1720-
| string.swift:635:13:635:13 | String.Type | string.swift:635:13:635:20 | .availableStringEncodings |
1721-
| string.swift:636:13:636:13 | String.Type | string.swift:636:13:636:20 | .defaultCStringEncoding |
1722-
| string.swift:637:13:637:13 | tainted | string.swift:637:13:637:21 | .isContiguousUTF8 |
17231709
| subscript.swift:1:7:1:7 | SSA def(self) | subscript.swift:1:7:1:7 | self[return] |
17241710
| subscript.swift:1:7:1:7 | SSA def(self) | subscript.swift:1:7:1:7 | self[return] |
17251711
| subscript.swift:1:7:1:7 | self | subscript.swift:1:7:1:7 | SSA def(self) |

‎swift/ql/test/library-tests/dataflow/taint/Taint.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,6 @@ edges
607607
| string.swift:331:14:331:22 | call to source2() : | string.swift:332:13:332:13 | str6 |
608608
| string.swift:331:14:331:22 | call to source2() : | string.swift:334:13:334:13 | str6 |
609609
| string.swift:540:17:540:25 | call to source2() : | string.swift:545:13:545:13 | sub1 |
610-
| string.swift:633:17:633:25 | call to source2() : | string.swift:637:13:637:21 | .isContiguousUTF8 |
611610
| subscript.swift:13:15:13:22 | call to source() : | subscript.swift:13:15:13:25 | ...[...] |
612611
| subscript.swift:14:15:14:23 | call to source2() : | subscript.swift:14:15:14:26 | ...[...] |
613612
| try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... |
@@ -1397,8 +1396,6 @@ nodes
13971396
| string.swift:540:17:540:25 | call to source2() : | semmle.label | call to source2() : |
13981397
| string.swift:542:13:542:21 | call to source7() | semmle.label | call to source7() |
13991398
| string.swift:545:13:545:13 | sub1 | semmle.label | sub1 |
1400-
| string.swift:633:17:633:25 | call to source2() : | semmle.label | call to source2() : |
1401-
| string.swift:637:13:637:21 | .isContiguousUTF8 | semmle.label | .isContiguousUTF8 |
14021399
| subscript.swift:13:15:13:22 | call to source() : | semmle.label | call to source() : |
14031400
| subscript.swift:13:15:13:25 | ...[...] | semmle.label | ...[...] |
14041401
| subscript.swift:14:15:14:23 | call to source2() : | semmle.label | call to source2() : |
@@ -1890,7 +1887,6 @@ subpaths
18901887
| string.swift:334:13:334:13 | str6 | string.swift:331:14:331:22 | call to source2() : | string.swift:334:13:334:13 | str6 | result |
18911888
| string.swift:542:13:542:21 | call to source7() | string.swift:542:13:542:21 | call to source7() | string.swift:542:13:542:21 | call to source7() | result |
18921889
| string.swift:545:13:545:13 | sub1 | string.swift:540:17:540:25 | call to source2() : | string.swift:545:13:545:13 | sub1 | result |
1893-
| string.swift:637:13:637:21 | .isContiguousUTF8 | string.swift:633:17:633:25 | call to source2() : | string.swift:637:13:637:21 | .isContiguousUTF8 | result |
18941890
| subscript.swift:13:15:13:25 | ...[...] | subscript.swift:13:15:13:22 | call to source() : | subscript.swift:13:15:13:25 | ...[...] | result |
18951891
| subscript.swift:14:15:14:26 | ...[...] | subscript.swift:14:15:14:23 | call to source2() : | subscript.swift:14:15:14:26 | ...[...] | result |
18961892
| try.swift:9:13:9:24 | try ... | try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... | result |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,5 +634,5 @@ func untaintedFields() {
634634

635635
sink(arg: String.availableStringEncodings)
636636
sink(arg: String.defaultCStringEncoding)
637-
sink(arg: tainted.isContiguousUTF8) // $ SPURIOUS: tainted=633
637+
sink(arg: tainted.isContiguousUTF8)
638638
}

0 commit comments

Comments
 (0)