Skip to content

Commit bc03f69

Browse files
geoffw0geoffw0
committed
Swift: Detect String -> NSString results.
1 parent a306f31 commit bc03f69

File tree

3 files changed

+59
-6
lines changed

3 files changed

+59
-6
lines changed

‎swift/ql/src/queries/Security/CWE-135/StringLengthConflation.ql

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ class StringLengthConflationConfiguration extends DataFlow::Configuration {
3030
node.asExpr() = member and
3131
flowstate = "String"
3232
)
33+
or
34+
// result of a call to to `NSString.length`
35+
exists(MemberRefExpr member |
36+
member.getBaseExpr().getType().getName() = ["NSString", "NSMutableString"] and
37+
member.getMember().(VarDecl).getName() = "length" and
38+
node.asExpr() = member and
39+
flowstate = "NSString"
40+
)
3341
}
3442

3543
override predicate isSink(DataFlow::Node node, string flowstate) {
@@ -83,6 +91,27 @@ class StringLengthConflationConfiguration extends DataFlow::Configuration {
8391
call.getArgument(pragma[only_bind_into](arg)).getExpr() = node.asExpr() and
8492
flowstate = "String" // `String` length flowing into `NSString`
8593
)
94+
or
95+
// arguments to function calls...
96+
exists(string funcName, string paramName, CallExpr call, int arg |
97+
(
98+
// `dropFirst`, `dropLast`, `removeFirst`, `removeLast`
99+
funcName = ["dropFirst(_:)", "dropLast(_:)", "removeFirst(_:)", "removeLast(_:)"] and
100+
paramName = "k"
101+
or
102+
// `prefix`, `suffix`
103+
funcName = ["prefix(_:)", "suffix(_:)"] and
104+
paramName = "maxLength"
105+
) and
106+
call.getFunction().(ApplyExpr).getStaticTarget().getName() = funcName and
107+
call.getFunction()
108+
.(ApplyExpr)
109+
.getStaticTarget()
110+
.getParam(pragma[only_bind_into](arg))
111+
.getName() = paramName and
112+
call.getArgument(pragma[only_bind_into](arg)).getExpr() = node.asExpr() and
113+
flowstate = "NSString" // `NSString` length flowing into `String`
114+
)
86115
}
87116

88117
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {

‎swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.expected

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
edges
2+
| StringLengthConflation.swift:93:28:93:31 | .length : | StringLengthConflation.swift:93:28:93:40 | ... call to -(_:_:) ... |
3+
| StringLengthConflation.swift:97:27:97:30 | .length : | StringLengthConflation.swift:97:27:97:39 | ... call to -(_:_:) ... |
4+
| StringLengthConflation.swift:101:25:101:28 | .length : | StringLengthConflation.swift:101:25:101:37 | ... call to -(_:_:) ... |
5+
| StringLengthConflation.swift:105:25:105:28 | .length : | StringLengthConflation.swift:105:25:105:37 | ... call to -(_:_:) ... |
6+
| StringLengthConflation.swift:111:23:111:26 | .length : | StringLengthConflation.swift:111:23:111:35 | ... call to -(_:_:) ... |
7+
| StringLengthConflation.swift:117:22:117:25 | .length : | StringLengthConflation.swift:117:22:117:34 | ... call to -(_:_:) ... |
28
| StringLengthConflation.swift:122:34:122:36 | .count : | StringLengthConflation.swift:122:34:122:44 | ... call to -(_:_:) ... |
39
| StringLengthConflation.swift:123:36:123:38 | .count : | StringLengthConflation.swift:123:36:123:46 | ... call to -(_:_:) ... |
410
| StringLengthConflation.swift:128:36:128:38 | .count : | StringLengthConflation.swift:128:36:128:46 | ... call to -(_:_:) ... |
@@ -9,6 +15,18 @@ edges
915
nodes
1016
| StringLengthConflation.swift:72:33:72:35 | .count | semmle.label | .count |
1117
| StringLengthConflation.swift:78:47:78:49 | .count | semmle.label | .count |
18+
| StringLengthConflation.swift:93:28:93:31 | .length : | semmle.label | .length : |
19+
| StringLengthConflation.swift:93:28:93:40 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
20+
| StringLengthConflation.swift:97:27:97:30 | .length : | semmle.label | .length : |
21+
| StringLengthConflation.swift:97:27:97:39 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
22+
| StringLengthConflation.swift:101:25:101:28 | .length : | semmle.label | .length : |
23+
| StringLengthConflation.swift:101:25:101:37 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
24+
| StringLengthConflation.swift:105:25:105:28 | .length : | semmle.label | .length : |
25+
| StringLengthConflation.swift:105:25:105:37 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
26+
| StringLengthConflation.swift:111:23:111:26 | .length : | semmle.label | .length : |
27+
| StringLengthConflation.swift:111:23:111:35 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
28+
| StringLengthConflation.swift:117:22:117:25 | .length : | semmle.label | .length : |
29+
| StringLengthConflation.swift:117:22:117:34 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
1230
| StringLengthConflation.swift:122:34:122:36 | .count : | semmle.label | .count : |
1331
| StringLengthConflation.swift:122:34:122:44 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
1432
| StringLengthConflation.swift:123:36:123:38 | .count : | semmle.label | .count : |
@@ -27,6 +45,12 @@ subpaths
2745
#select
2846
| StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:72:33:72:35 | .count | StringLengthConflation.swift:72:33:72:35 | .count | This String length is used in an NSString, but it may not be equivalent. |
2947
| StringLengthConflation.swift:78:47:78:49 | .count | StringLengthConflation.swift:78:47:78:49 | .count | StringLengthConflation.swift:78:47:78:49 | .count | This String length is used in an NSString, but it may not be equivalent. |
48+
| StringLengthConflation.swift:93:28:93:40 | ... call to -(_:_:) ... | StringLengthConflation.swift:93:28:93:31 | .length : | StringLengthConflation.swift:93:28:93:40 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
49+
| StringLengthConflation.swift:97:27:97:39 | ... call to -(_:_:) ... | StringLengthConflation.swift:97:27:97:30 | .length : | StringLengthConflation.swift:97:27:97:39 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
50+
| StringLengthConflation.swift:101:25:101:37 | ... call to -(_:_:) ... | StringLengthConflation.swift:101:25:101:28 | .length : | StringLengthConflation.swift:101:25:101:37 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
51+
| StringLengthConflation.swift:105:25:105:37 | ... call to -(_:_:) ... | StringLengthConflation.swift:105:25:105:28 | .length : | StringLengthConflation.swift:105:25:105:37 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
52+
| StringLengthConflation.swift:111:23:111:35 | ... call to -(_:_:) ... | StringLengthConflation.swift:111:23:111:26 | .length : | StringLengthConflation.swift:111:23:111:35 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
53+
| StringLengthConflation.swift:117:22:117:34 | ... call to -(_:_:) ... | StringLengthConflation.swift:117:22:117:25 | .length : | StringLengthConflation.swift:117:22:117:34 | ... call to -(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
3054
| StringLengthConflation.swift:122:34:122:44 | ... call to -(_:_:) ... | StringLengthConflation.swift:122:34:122:36 | .count : | StringLengthConflation.swift:122:34:122:44 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
3155
| StringLengthConflation.swift:123:36:123:46 | ... call to -(_:_:) ... | StringLengthConflation.swift:123:36:123:38 | .count : | StringLengthConflation.swift:123:36:123:46 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
3256
| StringLengthConflation.swift:128:36:128:46 | ... call to -(_:_:) ... | StringLengthConflation.swift:128:36:128:38 | .count : | StringLengthConflation.swift:128:36:128:46 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |

‎swift/ql/test/query-tests/Security/CWE-135/StringLengthConflation.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,31 +90,31 @@ func test(s: String) {
9090
// --- String operations using an integer directly ---
9191

9292
let str1 = s.dropFirst(s.count - 1) // GOOD
93-
let str2 = s.dropFirst(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
93+
let str2 = s.dropFirst(ns.length - 1) // BAD: NSString length used in String
9494
print("dropFirst '\(str1)' / '\(str2)'")
9595

9696
let str3 = s.dropLast(s.count - 1) // GOOD
97-
let str4 = s.dropLast(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
97+
let str4 = s.dropLast(ns.length - 1) // BAD: NSString length used in String
9898
print("dropLast '\(str3)' / '\(str4)'")
9999

100100
let str5 = s.prefix(s.count - 1) // GOOD
101-
let str6 = s.prefix(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
101+
let str6 = s.prefix(ns.length - 1) // BAD: NSString length used in String
102102
print("prefix '\(str5)' / '\(str6)'")
103103

104104
let str7 = s.suffix(s.count - 1) // GOOD
105-
let str8 = s.suffix(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
105+
let str8 = s.suffix(ns.length - 1) // BAD: NSString length used in String
106106
print("suffix '\(str7)' / '\(str8)'")
107107

108108
var str9 = s
109109
str9.removeFirst(s.count - 1) // GOOD
110110
var str10 = s
111-
str10.removeFirst(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
111+
str10.removeFirst(ns.length - 1) // BAD: NSString length used in String
112112
print("removeFirst '\(str9)' / '\(str10)'")
113113

114114
var str11 = s
115115
str11.removeLast(s.count - 1) // GOOD
116116
var str12 = s
117-
str12.removeLast(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
117+
str12.removeLast(ns.length - 1) // BAD: NSString length used in String
118118
print("removeLast '\(str11)' / '\(str12)'")
119119

120120
let nstr1 = ns.character(at: ns.length - 1) // GOOD

0 commit comments

Comments
 (0)