Swift: Support String.Index and flow through * /.

Author: geoffw0

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

@@ -95,17 +95,25 @@ class StringLengthConflationConfiguration extends DataFlow::Configuration {
     // arguments to function calls...
     exists(string funcName, string paramName, CallExpr call, int arg |
       (
-        // `dropFirst`, `dropLast`, `removeFirst`, `removeLast`
+        // `String.dropFirst`, `String.dropLast`, `String.removeFirst`, `String.removeLast`
         funcName = ["dropFirst(_:)", "dropLast(_:)", "removeFirst(_:)", "removeLast(_:)"] and
         paramName = "k"
         or
-        // `prefix`, `suffix`
+        // `String.prefix`, `String.suffix`
         funcName = ["prefix(_:)", "suffix(_:)"] and
         paramName = "maxLength"
         or
         // `String.Index.init`
         funcName = "init(encodedOffset:)" and
         paramName = "offset"
+        or
+        // `String.index`
+        funcName = ["index(_:offsetBy:)", "index(_:offsetBy:limitBy:)"] and
+        paramName = "n"
+        or
+        // `String.formIndex`
+        funcName = ["formIndex(_:offsetBy:)", "formIndex(_:offsetBy:limitBy:)"] and
+        paramName = "distance"
       ) and
       call.getFunction().(ApplyExpr).getStaticTarget().getName() = funcName and
       call.getFunction()
@@ -119,9 +127,8 @@ class StringLengthConflationConfiguration extends DataFlow::Configuration {
   }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    // allow flow through `+` and `-`.
-    node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr() or
-    node2.asExpr().(SubExpr).getAnOperand() = node1.asExpr()
+    // allow flow through `+`, `-`, `*` etc.
+    node2.asExpr().(ArithmeticOperation).getAnOperand() = node1.asExpr()
   }
 }
 

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

@@ -1,4 +1,6 @@
 edges
+| StringLengthConflation.swift:60:47:60:50 | .length :  | StringLengthConflation.swift:60:47:60:59 | ... call to /(_:_:) ... |
+| StringLengthConflation.swift:66:33:66:36 | .length :  | StringLengthConflation.swift:66:33:66:45 | ... call to /(_:_:) ... |
 | StringLengthConflation.swift:93:28:93:31 | .length :  | StringLengthConflation.swift:93:28:93:40 | ... call to -(_:_:) ... |
 | StringLengthConflation.swift:97:27:97:30 | .length :  | StringLengthConflation.swift:97:27:97:39 | ... call to -(_:_:) ... |
 | StringLengthConflation.swift:101:25:101:28 | .length :  | StringLengthConflation.swift:101:25:101:37 | ... call to -(_:_:) ... |
@@ -14,6 +16,10 @@ edges
 | StringLengthConflation.swift:141:28:141:30 | .count :  | StringLengthConflation.swift:141:28:141:38 | ... call to -(_:_:) ... |
 nodes
 | StringLengthConflation.swift:53:43:53:46 | .length | semmle.label | .length |
+| StringLengthConflation.swift:60:47:60:50 | .length :  | semmle.label | .length :  |
+| StringLengthConflation.swift:60:47:60:59 | ... call to /(_:_:) ... | semmle.label | ... call to /(_:_:) ... |
+| StringLengthConflation.swift:66:33:66:36 | .length :  | semmle.label | .length :  |
+| StringLengthConflation.swift:66:33:66:45 | ... call to /(_:_:) ... | semmle.label | ... call to /(_:_:) ... |
 | StringLengthConflation.swift:72:33:72:35 | .count | semmle.label | .count |
 | StringLengthConflation.swift:78:47:78:49 | .count | semmle.label | .count |
 | StringLengthConflation.swift:93:28:93:31 | .length :  | semmle.label | .length :  |
@@ -45,6 +51,8 @@ nodes
 subpaths
 #select
 | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | StringLengthConflation.swift:53:43:53:46 | .length | This NSString length is used in a String, but it may not be equivalent. |
+| StringLengthConflation.swift:60:47:60:59 | ... call to /(_:_:) ... | StringLengthConflation.swift:60:47:60:50 | .length :  | StringLengthConflation.swift:60:47:60:59 | ... call to /(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
+| StringLengthConflation.swift:66:33:66:45 | ... call to /(_:_:) ... | StringLengthConflation.swift:66:33:66:36 | .length :  | StringLengthConflation.swift:66:33:66:45 | ... call to /(_:_:) ... | This NSString length is used in a String, but it may not be equivalent. |
 | 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. |
 | 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. |
 | 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. |

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

@@ -57,13 +57,13 @@ func test(s: String) {
     print("String.Index '\(ix1.encodedOffset)' / '\(ix2.encodedOffset)' '\(ix3.encodedOffset)' '\(ix4.encodedOffset)' '\(ix5.encodedOffset)'")
 
     let ix6 = s.index(s.startIndex, offsetBy: s.count / 2) // GOOD
-    let ix7 = s.index(s.startIndex, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index [NOT DETECTED]
+    let ix7 = s.index(s.startIndex, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index
     print("index '\(ix6.encodedOffset)' / '\(ix7.encodedOffset)'")
 
     var ix8 = s.startIndex
     s.formIndex(&ix8, offsetBy: s.count / 2) // GOOD
     var ix9 = s.startIndex
-    s.formIndex(&ix9, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index [NOT DETECTED]
+    s.formIndex(&ix9, offsetBy: ns.length / 2) // BAD: NSString length used in String.Index
     print("formIndex '\(ix8.encodedOffset)' / '\(ix9.encodedOffset)'")
 
     // --- constructing an NSRange from integers ---