Skip to content

Commit 6b5785b

Browse files
MathiasVPMathiasVP
authored
Merge pull request #9765 from geoffw0/stringlengthconflation3
Swift: Improvements to the string length conflation query
2 parents 28c05e7 + 68c7600 commit 6b5785b

File tree

3 files changed

+51
-12
lines changed

3 files changed

+51
-12
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ import swift
1414
import codeql.swift.dataflow.DataFlow
1515
import DataFlow::PathGraph
1616

17+
/**
18+
* A configuration for tracking string lengths originating from source that is
19+
* a `String` or an `NSString` object, to a sink of a different kind that
20+
* expects an incompatible measure of length.
21+
*/
1722
class StringLengthConflationConfiguration extends DataFlow::Configuration {
1823
StringLengthConflationConfiguration() { this = "StringLengthConflationConfiguration" }
1924

@@ -79,6 +84,12 @@ class StringLengthConflationConfiguration extends DataFlow::Configuration {
7984
flowstate = "String" // `String` length flowing into `NSString`
8085
)
8186
}
87+
88+
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
89+
// allow flow through `+` and `-`.
90+
node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr() or
91+
node2.asExpr().(SubExpr).getAnOperand() = node1.asExpr()
92+
}
8293
}
8394

8495
from

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,36 @@
11
edges
2+
| StringLengthConflation.swift:101:34:101:36 | .count : | StringLengthConflation.swift:101:34:101:44 | ... call to -(_:_:) ... |
3+
| StringLengthConflation.swift:102:36:102:38 | .count : | StringLengthConflation.swift:102:36:102:46 | ... call to -(_:_:) ... |
4+
| StringLengthConflation.swift:107:36:107:38 | .count : | StringLengthConflation.swift:107:36:107:46 | ... call to -(_:_:) ... |
5+
| StringLengthConflation.swift:108:38:108:40 | .count : | StringLengthConflation.swift:108:38:108:48 | ... call to -(_:_:) ... |
6+
| StringLengthConflation.swift:113:34:113:36 | .count : | StringLengthConflation.swift:113:34:113:44 | ... call to -(_:_:) ... |
7+
| StringLengthConflation.swift:114:36:114:38 | .count : | StringLengthConflation.swift:114:36:114:46 | ... call to -(_:_:) ... |
8+
| StringLengthConflation.swift:120:28:120:30 | .count : | StringLengthConflation.swift:120:28:120:38 | ... call to -(_:_:) ... |
29
nodes
310
| StringLengthConflation.swift:72:33:72:35 | .count | semmle.label | .count |
411
| StringLengthConflation.swift:78:47:78:49 | .count | semmle.label | .count |
12+
| StringLengthConflation.swift:101:34:101:36 | .count : | semmle.label | .count : |
13+
| StringLengthConflation.swift:101:34:101:44 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
14+
| StringLengthConflation.swift:102:36:102:38 | .count : | semmle.label | .count : |
15+
| StringLengthConflation.swift:102:36:102:46 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
16+
| StringLengthConflation.swift:107:36:107:38 | .count : | semmle.label | .count : |
17+
| StringLengthConflation.swift:107:36:107:46 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
18+
| StringLengthConflation.swift:108:38:108:40 | .count : | semmle.label | .count : |
19+
| StringLengthConflation.swift:108:38:108:48 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
20+
| StringLengthConflation.swift:113:34:113:36 | .count : | semmle.label | .count : |
21+
| StringLengthConflation.swift:113:34:113:44 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
22+
| StringLengthConflation.swift:114:36:114:38 | .count : | semmle.label | .count : |
23+
| StringLengthConflation.swift:114:36:114:46 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
24+
| StringLengthConflation.swift:120:28:120:30 | .count : | semmle.label | .count : |
25+
| StringLengthConflation.swift:120:28:120:38 | ... call to -(_:_:) ... | semmle.label | ... call to -(_:_:) ... |
526
subpaths
627
#select
728
| 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. |
829
| 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. |
30+
| StringLengthConflation.swift:101:34:101:44 | ... call to -(_:_:) ... | StringLengthConflation.swift:101:34:101:36 | .count : | StringLengthConflation.swift:101:34:101:44 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
31+
| StringLengthConflation.swift:102:36:102:46 | ... call to -(_:_:) ... | StringLengthConflation.swift:102:36:102:38 | .count : | StringLengthConflation.swift:102:36:102:46 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
32+
| StringLengthConflation.swift:107:36:107:46 | ... call to -(_:_:) ... | StringLengthConflation.swift:107:36:107:38 | .count : | StringLengthConflation.swift:107:36:107:46 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
33+
| StringLengthConflation.swift:108:38:108:48 | ... call to -(_:_:) ... | StringLengthConflation.swift:108:38:108:40 | .count : | StringLengthConflation.swift:108:38:108:48 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
34+
| StringLengthConflation.swift:113:34:113:44 | ... call to -(_:_:) ... | StringLengthConflation.swift:113:34:113:36 | .count : | StringLengthConflation.swift:113:34:113:44 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
35+
| StringLengthConflation.swift:114:36:114:46 | ... call to -(_:_:) ... | StringLengthConflation.swift:114:36:114:38 | .count : | StringLengthConflation.swift:114:36:114:46 | ... call to -(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
36+
| StringLengthConflation.swift:120:28:120:38 | ... call to -(_:_:) ... | StringLengthConflation.swift:120:28:120:30 | .count : | StringLengthConflation.swift:120:28:120:38 | ... 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: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,28 +50,28 @@ func test(s: String) {
5050
// --- constructing a String.Index from integer ---
5151

5252
let ix1 = String.Index(encodedOffset: s.count) // GOOD
53-
let ix2 = String.Index(encodedOffset: ns.length) // BAD: NSString length used in String.Index
54-
let ix3 = String.Index(encodedOffset: s.utf8.count) // BAD: String.utf8 length used in String.Index
55-
let ix4 = String.Index(encodedOffset: s.utf16.count) // BAD: String.utf16 length used in String.Index
56-
let ix5 = String.Index(encodedOffset: s.unicodeScalars.count) // BAD: string.unicodeScalars length used in String.Index
53+
let ix2 = String.Index(encodedOffset: ns.length) // BAD: NSString length used in String.Index [NOT DETECTED]
54+
let ix3 = String.Index(encodedOffset: s.utf8.count) // BAD: String.utf8 length used in String.Index [NOT DETECTED]
55+
let ix4 = String.Index(encodedOffset: s.utf16.count) // BAD: String.utf16 length used in String.Index [NOT DETECTED]
56+
let ix5 = String.Index(encodedOffset: s.unicodeScalars.count) // BAD: string.unicodeScalars length used in String.Index [NOT DETECTED]
5757
print("String.Index '\(ix1.encodedOffset)' / '\(ix2.encodedOffset)' '\(ix3.encodedOffset)' '\(ix4.encodedOffset)' '\(ix5.encodedOffset)'")
5858

5959
let ix6 = s.index(s.startIndex, offsetBy: s.count / 2) // GOOD
60-
let ix7 = s.index(s.startIndex, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index
60+
let ix7 = s.index(s.startIndex, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index [NOT DETECTED]
6161
print("index '\(ix6.encodedOffset)' / '\(ix7.encodedOffset)'")
6262

6363
var ix8 = s.startIndex
6464
s.formIndex(&ix8, offsetBy: s.count / 2) // GOOD
6565
var ix9 = s.startIndex
66-
s.formIndex(&ix9, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index
66+
s.formIndex(&ix9, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index [NOT DETECTED]
6767
print("formIndex '\(ix8.encodedOffset)' / '\(ix9.encodedOffset)'")
6868

6969
// --- constructing an NSRange from integers ---
7070

7171
let range1 = NSMakeRange(0, ns.length) // GOOD
7272
let range2 = NSMakeRange(0, s.count) // BAD: String length used in NSMakeRange
73-
let range3 = NSMakeRange(0, s.reversed().count) // BAD: String length used in NSMakeRange
74-
let range4 = NSMakeRange(0, s.distance(from: s.startIndex, to: s.endIndex)) // BAD: String length used in NSMakeRange
73+
let range3 = NSMakeRange(0, s.reversed().count) // BAD: String length used in NSMakeRange [NOT DETECTED]
74+
let range4 = NSMakeRange(0, s.distance(from: s.startIndex, to: s.endIndex)) // BAD: String length used in NSMakeRange [NOT DETECTED]
7575
print("NSMakeRange '\(range1.description)' / '\(range2.description)' '\(range3.description)' '\(range4.description)'")
7676

7777
let range5 = NSRange(location: 0, length: ns.length) // GOOD
@@ -81,19 +81,19 @@ func test(s: String) {
8181
// --- String operations using an integer directly ---
8282

8383
let str1 = s.dropFirst(s.count - 1) // GOOD
84-
let str2 = s.dropFirst(ns.length - 1) // BAD: NSString length used in String
84+
let str2 = s.dropFirst(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
8585
print("dropFirst '\(str1)' / '\(str2)'")
8686

8787
let str3 = s.dropLast(s.count - 1) // GOOD
88-
let str4 = s.dropLast(ns.length - 1) // BAD: NSString length used in String
88+
let str4 = s.dropLast(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
8989
print("dropLast '\(str3)' / '\(str4)'")
9090

9191
let str5 = s.prefix(s.count - 1) // GOOD
92-
let str6 = s.prefix(ns.length - 1) // BAD: NSString length used in String
92+
let str6 = s.prefix(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
9393
print("prefix '\(str5)' / '\(str6)'")
9494

9595
let str7 = s.suffix(s.count - 1) // GOOD
96-
let str8 = s.suffix(ns.length - 1) // BAD: NSString length used in String
96+
let str8 = s.suffix(ns.length - 1) // BAD: NSString length used in String [NOT DETECTED]
9797
print("suffix '\(str7)' / '\(str8)'")
9898

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

0 commit comments

Comments
 (0)