Merge pull request github#12416 from geoffw0/contig

Swift: Model Sequence.withContiguousStorageIfAvailable

Author: geoffw0

swift/ql/lib/change-notes/2023-07-24-with-contiguous-storage.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added flow models for `Sequence.withContiguousStorageIfAvailable`.

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

@@ -25,6 +25,8 @@ private class SequenceSummaries extends SummaryModelCsv {
         ";Sequence;true;joined();;;Argument[-1];ReturnValue;taint",
         ";Sequence;true;joined(separator:);;;Argument[-1..0];ReturnValue;taint",
         ";Sequence;true;first(where:);;;Argument[-1];ReturnValue;taint",
+        ";Sequence;true;withContiguousStorageIfAvailable(_:);;;Argument[-1];Argument[0].Parameter[0];taint",
+        ";Sequence;true;withContiguousStorageIfAvailable(_:);;;Argument[0].ReturnValue;ReturnValue;taint",
       ]
   }
 }

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

@@ -118,9 +118,7 @@ private class DefaultUnsafeJsEvalAdditionalFlowStep extends UnsafeJsEvalAddition
     )
     or
     exists(CallExpr ce, Expr self, ClosureExpr closure |
-      ce.getStaticTarget()
-          .getName()
-          .matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
+      ce.getStaticTarget().getName().matches("withUnsafeBufferPointer(%)") and
       self = ce.getQualifier() and
       ce.getArgument(0).getExpr() = closure
     |

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

@@ -438,7 +438,7 @@ func taintThroughEncodings() {
   })
   tainted.withContiguousStorageIfAvailable({
     ptr in
-    sink(arg: ptr)
+    sink(arg: ptr) // $ tainted=366
     sink(arg: ptr.baseAddress!) // $ MISSING: tainted=366
   })
 }
@@ -599,3 +599,52 @@ func untaintedFields() {
   sink(arg: String.defaultCStringEncoding)
   sink(arg: tainted.isContiguousUTF8)
 }
+
+func callbackWithCleanPointer(ptr: UnsafeBufferPointer<String.Element>) throws -> Int {
+  sink(arg: ptr)
+
+  return 0
+}
+
+func callbackWithTaintedPointer(ptr: UnsafeBufferPointer<String.Element>) throws -> Int {
+  sink(arg: ptr) // $ tainted=617
+
+  return source()
+}
+
+func furtherTaintThroughCallbacks() {
+  let clean = ""
+  let tainted = source2()
+
+  // return values from the closure (1)
+  let result1 = clean.withContiguousStorageIfAvailable({
+    ptr in
+    return 0
+  })
+  sink(arg: result1!)
+  let result2 = clean.withContiguousStorageIfAvailable({
+    ptr in
+    return source()
+  })
+  sink(arg: result2!) // $ tainted=627
+
+  // return values from the closure (2)
+  if let result3 = clean.withContiguousStorageIfAvailable({
+    ptr in
+    return 0
+  }) {
+    sink(arg: result3)
+  }
+  if let result4 = clean.withContiguousStorageIfAvailable({
+    ptr in
+    return source()
+  }) {
+    sink(arg: result4) // $ MISSING: tainted=640
+  }
+
+  // using a non-closure function
+  let result5 = try? clean.withContiguousStorageIfAvailable(callbackWithCleanPointer)
+  sink(arg: result5!)
+  let result6 = try? tainted.withContiguousStorageIfAvailable(callbackWithTaintedPointer)
+  sink(arg: result6!) // $ tainted=612
+}