Swift: Replace sinks with (extendable) CSV.

Author: geoffw0

swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll

@@ -16,7 +16,8 @@
  * 1. The `namespace` column selects a package.
  * 2. The `type` column selects a type within that package.
  * 3. The `subtypes` is a boolean that indicates whether to jump to an
- *    arbitrary subtype of that type.
+ *    arbitrary subtype of that type. Set this to `false` if leaving the `type`
+ *    blank (for example, a free function).
  * 4. The `name` column optionally selects a specific named member of the type.
  * 5. The `signature` column optionally restricts the named member. If
  *    `signature` is blank then no such filtering is done. The format of the

swift/ql/lib/codeql/swift/security/StringLengthConflationExtensions.qll

@@ -5,6 +5,7 @@
 
 import swift
 import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.ExternalFlow
 
 /**
  * A type of Swift string encoding. This class is used as a flow state for
@@ -13,31 +14,50 @@ import codeql.swift.dataflow.DataFlow
 class StringType extends string {
   string singular;
   string equivClass;
+  string csvLabel;
 
   StringType() {
-    this = "String" and singular = "a String" and equivClass = "String"
+    this = "String" and
+    singular = "a String" and
+    equivClass = "String" and
+    csvLabel = "string-length"
     or
-    this = "NSString" and singular = "an NSString" and equivClass = "NSString"
+    this = "NSString" and
+    singular = "an NSString" and
+    equivClass = "NSString" and
+    csvLabel = "nsstring-length"
     or
-    this = "String.utf8" and singular = "a String.utf8" and equivClass = "String.utf8"
+    this = "String.utf8" and
+    singular = "a String.utf8" and
+    equivClass = "String.utf8" and
+    csvLabel = "string-utf8-length"
     or
-    this = "String.utf16" and singular = "a String.utf16" and equivClass = "NSString"
+    this = "String.utf16" and
+    singular = "a String.utf16" and
+    equivClass = "NSString" and
+    csvLabel = "string-utf16-length"
     or
     this = "String.unicodeScalars" and
     singular = "a String.unicodeScalars" and
-    equivClass = "String.unicodeScalars"
+    equivClass = "String.unicodeScalars" and
+    csvLabel = "string-unicodescalars-length"
   }
 
   /**
-   * Gets the equivalence class for this flow state. If these are equal,
+   * Gets the equivalence class for this string type. If these are equal,
    * they should be treated as equivalent.
    */
   string getEquivClass() { result = equivClass }
 
   /**
-   * Gets text for the singular form of this flow state.
+   * Gets text for the singular form of this string type.
    */
   string getSingular() { result = singular }
+
+  /**
+   * Gets the label for this string type in CSV models.
+   */
+  string getCsvLabel() { result = csvLabel }
 }
 
 /**
@@ -115,83 +135,41 @@ private class DefaultStringLengthConflationSource extends StringLengthConflation
   override StringType getStringType() { result = stringType }
 }
 
+/**
+ * A sink defined in a CSV model.
+ */
 private class DefaultStringLengthConflationSink extends StringLengthConflationSink {
-  StringType correctStringType;
+  StringType stringType;
 
-  DefaultStringLengthConflationSink() {
-    exists(AbstractFunctionDecl funcDecl, CallExpr call, string funcName, int arg |
-      (
-        // arguments to method calls...
-        exists(string className, ClassOrStructDecl c |
-          (
-            // `NSRange.init`
-            className = "NSRange" and
-            funcName = "init(location:length:)" and
-            arg = [0, 1]
-            or
-            // `NSString.character`
-            className = ["NSString", "NSMutableString"] and
-            funcName = "character(at:)" and
-            arg = 0
-            or
-            // `NSString.character`
-            className = ["NSString", "NSMutableString"] and
-            funcName = "substring(from:)" and
-            arg = 0
-            or
-            // `NSString.character`
-            className = ["NSString", "NSMutableString"] and
-            funcName = "substring(to:)" and
-            arg = 0
-            or
-            // `NSMutableString.insert`
-            className = "NSMutableString" and
-            funcName = "insert(_:at:)" and
-            arg = 1
-          ) and
-          c.getName() = className and
-          c.getABaseTypeDecl*().(ClassOrStructDecl).getAMember() = funcDecl and
-          call.getStaticTarget() = funcDecl and
-          correctStringType = "NSString"
-        )
-        or
-        // arguments to function calls...
-        // `NSMakeRange`
-        funcName = "NSMakeRange(_:_:)" and
-        arg = [0, 1] and
-        call.getStaticTarget() = funcDecl and
-        correctStringType = "NSString"
-        or
-        // arguments to method calls...
-        (
-          // `String.dropFirst`, `String.dropLast`, `String.removeFirst`, `String.removeLast`
-          funcName = ["dropFirst(_:)", "dropLast(_:)", "removeFirst(_:)", "removeLast(_:)"] and
-          arg = 0
-          or
-          // `String.prefix`, `String.suffix`
-          funcName = ["prefix(_:)", "suffix(_:)"] and
-          arg = 0
-          or
-          // `String.Index.init`
-          funcName = "init(encodedOffset:)" and
-          arg = 0
-          or
-          // `String.index`
-          funcName = ["index(_:offsetBy:)", "index(_:offsetBy:limitBy:)"] and
-          arg = [0, 1]
-          or
-          // `String.formIndex`
-          funcName = ["formIndex(_:offsetBy:)", "formIndex(_:offsetBy:limitBy:)"] and
-          arg = [0, 1]
-        ) and
-        call.getStaticTarget() = funcDecl and
-        correctStringType = "String"
-      ) and
-      // match up `funcName`, `arg`, `node`.
-      funcDecl.getName() = funcName and
-      call.getArgument(arg).getExpr() = this.asExpr()
-    )
-  }
+  DefaultStringLengthConflationSink() { sinkNode(this, stringType.getCsvLabel()) }
 
-  override StringType getCorrectStringType() { result = correctStringType }
+  override StringType getCorrectStringType() { result = stringType }
+}
+
+private class StringLengthConflationSinks extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";Sequence;true;dropFirst(_:);;;Argument[0];string-length",
+        ";Sequence;true;dropLast(_:);;;Argument[0];string-length",
+        ";Sequence;true;prefix(_:);;;Argument[0];string-length",
+        ";Sequence;true;suffix(_:);;;Argument[0];string-length",
+        ";Collection;true;formIndex(_:offsetBy:);;;Argument[0..1];string-length",
+        ";Collection;true;formIndex(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
+        ";Collection;true;removeFirst(_:);;;Argument[0];string-length",
+        ";RangeReplaceableCollection;true;removeLast(_:);;;Argument[0];string-length",
+        ";String;true;index(_:offsetBy:);;;Argument[0..1];string-length",
+        ";String;true;index(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
+        ";String.Index;true;init(encodedOffset:);;;Argument[0];string-length",
+        ";NSRange;true;init(location:length:);;;Argument[0..1];nsstring-length",
+        ";NSString;true;character(at:);;;Argument[0];nsstring-length",
+        ";NSString;true;substring(from:);;;Argument[0];nsstring-length",
+        ";NSString;true;substring(to:);;;Argument[0];nsstring-length",
+        ";NSMutableString;true;character(at:);;;Argument[0];nsstring-length",
+        ";NSMutableString;true;substring(from:);;;Argument[0];nsstring-length",
+        ";NSMutableString;true;substring(to:);;;Argument[0];nsstring-length",
+        ";NSMutableString;true;insert(_:at:);;;Argument[1];nsstring-length",
+        ";;false;NSMakeRange(_:_:);;;Argument[0..1];nsstring-length",
+      ]
+  }
 }

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

@@ -1,5 +1,4 @@
 edges
-| StringLengthConflation2.swift:35:36:35:38 | .count :  | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... |
 | StringLengthConflation2.swift:37:34:37:36 | .count :  | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... |
 | StringLengthConflation.swift:36:30:36:37 | len :  | StringLengthConflation.swift:36:93:36:93 | len |
 | StringLengthConflation.swift:60:47:60:50 | .length :  | StringLengthConflation.swift:60:47:60:59 | ... ./(_:_:) ... |
@@ -28,8 +27,6 @@ edges
 | file://:0:0:0:0 | .length :  | StringLengthConflation.swift:114:23:114:26 | .length :  |
 | file://:0:0:0:0 | .length :  | StringLengthConflation.swift:120:22:120:25 | .length :  |
 nodes
-| StringLengthConflation2.swift:35:36:35:38 | .count :  | semmle.label | .count :  |
-| StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
 | StringLengthConflation2.swift:37:34:37:36 | .count :  | semmle.label | .count :  |
 | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
 | StringLengthConflation.swift:36:30:36:37 | len :  | semmle.label | len :  |
@@ -74,15 +71,11 @@ nodes
 | StringLengthConflation.swift:144:28:144:30 | .count :  | semmle.label | .count :  |
 | StringLengthConflation.swift:144:28:144:38 | ... .-(_:_:) ... | semmle.label | ... .-(_:_:) ... |
 | StringLengthConflation.swift:151:45:151:53 | .count | semmle.label | .count |
-| StringLengthConflation.swift:152:57:152:65 | .count | semmle.label | .count |
 | StringLengthConflation.swift:156:45:156:52 | .count | semmle.label | .count |
-| StringLengthConflation.swift:157:55:157:62 | .count | semmle.label | .count |
 | StringLengthConflation.swift:161:45:161:53 | .count | semmle.label | .count |
-| StringLengthConflation.swift:162:57:162:65 | .count | semmle.label | .count |
 | file://:0:0:0:0 | .length :  | semmle.label | .length :  |
 subpaths
 #select
-| StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | StringLengthConflation2.swift:35:36:35:38 | .count :  | StringLengthConflation2.swift:35:36:35:46 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
 | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | StringLengthConflation2.swift:37:34:37:36 | .count :  | StringLengthConflation2.swift:37:34:37:44 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
 | StringLengthConflation.swift:36:93:36:93 | len | StringLengthConflation.swift:72:33:72:35 | .count :  | StringLengthConflation.swift:36:93:36:93 | len | This String length is used in an NSString, but it may not be equivalent. |
 | 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. |
@@ -118,8 +111,5 @@ subpaths
 | StringLengthConflation.swift:138:36:138:46 | ... .-(_:_:) ... | StringLengthConflation.swift:138:36:138:38 | .count :  | StringLengthConflation.swift:138:36:138:46 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
 | StringLengthConflation.swift:144:28:144:38 | ... .-(_:_:) ... | StringLengthConflation.swift:144:28:144:30 | .count :  | StringLengthConflation.swift:144:28:144:38 | ... .-(_:_:) ... | This String length is used in an NSString, but it may not be equivalent. |
 | StringLengthConflation.swift:151:45:151:53 | .count | StringLengthConflation.swift:151:45:151:53 | .count | StringLengthConflation.swift:151:45:151:53 | .count | This String.unicodeScalars length is used in a String, but it may not be equivalent. |
-| StringLengthConflation.swift:152:57:152:65 | .count | StringLengthConflation.swift:152:57:152:65 | .count | StringLengthConflation.swift:152:57:152:65 | .count | This String.unicodeScalars length is used in a String, but it may not be equivalent. |
 | StringLengthConflation.swift:156:45:156:52 | .count | StringLengthConflation.swift:156:45:156:52 | .count | StringLengthConflation.swift:156:45:156:52 | .count | This String.utf8 length is used in a String, but it may not be equivalent. |
-| StringLengthConflation.swift:157:55:157:62 | .count | StringLengthConflation.swift:157:55:157:62 | .count | StringLengthConflation.swift:157:55:157:62 | .count | This String.utf8 length is used in a String, but it may not be equivalent. |
 | StringLengthConflation.swift:161:45:161:53 | .count | StringLengthConflation.swift:161:45:161:53 | .count | StringLengthConflation.swift:161:45:161:53 | .count | This String.utf16 length is used in a String, but it may not be equivalent. |
-| StringLengthConflation.swift:162:57:162:65 | .count | StringLengthConflation.swift:162:57:162:65 | .count | StringLengthConflation.swift:162:57:162:65 | .count | This String.unicodeScalars length is used in a String, but it may not be equivalent. |

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

@@ -149,17 +149,17 @@ func test(s: String) {
     let scalars = s.unicodeScalars
     let _ = s.index(s.startIndex, offsetBy: s.count) // GOOD
     let _ = s.index(s.startIndex, offsetBy: scalars.count) // BAD
-    let _ = scalars.index(scalars.startIndex, offsetBy: scalars.count) // GOOD [FALSE POSITIVE]
+    let _ = scalars.index(scalars.startIndex, offsetBy: scalars.count) // GOOD
     let _ = scalars.index(scalars.startIndex, offsetBy: s.count) // BAD [NOT DETECTED]
 
     let s_utf8 = s.utf8
     let _ = s.index(s.startIndex, offsetBy: s_utf8.count) // BAD
-    let _ = s_utf8.index(s_utf8.startIndex, offsetBy: s_utf8.count) // GOOD [FALSE POSITIVE]
+    let _ = s_utf8.index(s_utf8.startIndex, offsetBy: s_utf8.count) // GOOD
     let _ = s_utf8.index(s_utf8.startIndex, offsetBy: s.count) // BAD [NOT DETECTED]
 
     let s_utf16 = s.utf16
     let _ = s.index(s.startIndex, offsetBy: s_utf16.count) // BAD
-    let _ = s_utf16.index(s_utf16.startIndex, offsetBy: scalars.count) // GOOD [FALSE POSITIVE]
+    let _ = s_utf16.index(s_utf16.startIndex, offsetBy: scalars.count) // GOOD
     let _ = s_utf16.index(s_utf16.startIndex, offsetBy: s.count) // BAD [NOT DETECTED]
 }