Merge pull request #14511 from geoffw0/substring

Swift: Model Substring

Author: geoffw0

swift/ql/lib/change-notes/2023-10-16-substring.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added taint flow models for members of `Substring`.

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Collection.qll

@@ -36,6 +36,8 @@ private class CollectionSummaries extends SummaryModelCsv {
         ";RangeReplaceableCollection;true;removeFirst();;;Argument[-1];ReturnValue;taint",
         ";RangeReplaceableCollection;true;removeLast();;;Argument[-1];ReturnValue;taint",
         ";RangeReplaceableCollection;true;insert(_:at:);;;Argument[0];Argument[-1];taint",
+        ";RangeReplaceableCollection;true;replaceSubrange(_:with:);;;Argument[1];Argument[-1];taint",
+        ";RangeReplaceableCollection;true;replaceSubrange(_:with:);;;Argument[1].CollectionElement;Argument[-1].CollectionElement;value",
         ";BidirectionalCollection;true;joined(separator:);;;Argument[-1..0];ReturnValue;taint",
         ";BidirectionalCollection;true;last(where:);;;Argument[-1];ReturnValue;taint",
         ";BidirectionalCollection;true;popLast();;;Argument[-1];ReturnValue;taint",

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

@@ -113,7 +113,6 @@ private class StringSummaries extends SummaryModelCsv {
         ";String;true;localizedStringWithFormat(_:_:);;;Argument[0];ReturnValue;taint",
         ";String;true;localizedStringWithFormat(_:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
         ";String;true;insert(contentsOf:at:);;;Argument[0];Argument[-1];taint",
-        ";String;true;replaceSubrange(_:with:);;;Argument[1];Argument[-1];taint",
         ";String;true;max();;;Argument[-1];ReturnValue;taint",
         ";String;true;max(by:);;;Argument[-1];ReturnValue;taint",
         ";String;true;min();;;Argument[-1];ReturnValue;taint",
@@ -127,6 +126,9 @@ private class StringSummaries extends SummaryModelCsv {
         ";String;true;decodeCString(_:as:repairingInvalidCodeUnits:);;;Argument[0];ReturnValue.TupleElement[0];taint",
         ";String;true;decodeCString(_:as:repairingInvalidCodeUnits:);;;Argument[0].CollectionElement;ReturnValue.TupleElement[0];taint",
         ";LosslessStringConvertible;true;init(_:);;;Argument[0];ReturnValue;taint",
+        ";Substring;true;withUTF8(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
+        ";Substring;true;withUTF8(_:);;;Argument[0].Parameter[0].CollectionElement;Argument[-1];taint",
+        ";Substring;true;withUTF8(_:);;;Argument[0].ReturnValue;ReturnValue;value",
       ]
   }
 }
@@ -139,23 +141,26 @@ private class StringFieldsInheritTaint extends TaintInheritingContent,
   DataFlow::Content::FieldContent
 {
   StringFieldsInheritTaint() {
-    this.getField()
-        .hasQualifiedName(["String", "StringProtocol"],
+    exists(FieldDecl fieldDecl, Decl declaringDecl, TypeDecl namedTypeDecl |
+      (
+        namedTypeDecl.getFullName() = ["String", "StringProtocol"] and
+        fieldDecl.getName() =
           [
             "unicodeScalars", "utf8", "utf16", "lazy", "utf8CString", "dataValue",
             "identifierValue", "capitalized", "localizedCapitalized", "localizedLowercase",
             "localizedUppercase", "decomposedStringWithCanonicalMapping",
             "decomposedStringWithCompatibilityMapping", "precomposedStringWithCanonicalMapping",
             "precomposedStringWithCompatibilityMapping", "removingPercentEncoding"
-          ])
-    or
-    exists(FieldDecl fieldDecl, Decl declaringDecl, TypeDecl namedTypeDecl |
-      (
+          ]
+        or
         namedTypeDecl.getFullName() = "CustomStringConvertible" and
         fieldDecl.getName() = "description"
         or
         namedTypeDecl.getFullName() = "CustomDebugStringConvertible" and
         fieldDecl.getName() = "debugDescription"
+        or
+        namedTypeDecl.getFullName() = "Substring" and
+        fieldDecl.getName() = "base"
       ) and
       declaringDecl.getAMember() = fieldDecl and
       declaringDecl.asNominalTypeDecl() = namedTypeDecl.getADerivedTypeDecl*() and

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

@@ -687,3 +687,32 @@ func testDecodeCString() {
   sink(arg: str4) // $ tainted=669
   sink(arg: repaired4)
 }
+
+func testSubstringMembers() {
+  let clean = ""
+  let tainted = source2()
+
+  let sub1 = tainted[..<tainted.index(tainted.endIndex, offsetBy: -5)]
+  sink(arg: sub1) // $ tainted=693
+  sink(arg: sub1.base) // $ tainted=693
+  sink(arg: sub1.utf8) // $ tainted=693
+  sink(arg: sub1.capitalized) // $ tainted=693
+  sink(arg: sub1.description) // $ tainted=693
+
+  var sub2 = tainted[tainted.index(tainted.startIndex, offsetBy: 5)...]
+  sink(arg: sub2) // $ tainted=693
+  let result1 = sub2.withUTF8({
+    buffer in
+    sink(arg: buffer[0]) // $ tainted=693
+    return source()
+  })
+  sink(arg: result1) // $ tainted=707
+
+  let sub3 = Substring(sub2.utf8)
+  sink(arg: sub3) // $ tainted=693
+
+  var sub4 = clean.prefix(10)
+  sink(arg: sub4)
+  sub4.replaceSubrange(..<clean.endIndex, with: sub1)
+  sink(arg: sub4) // $ tainted=693
+}