Swift: add change note for unsafe closure query

swift/ql/src/change-notes/2023-07-13-unsafe-pointer-escapes-closure.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `swift/unsafe-pointer-escapes-closure` to detect code that passes temporary closure arguments outside the closure.

swift/ql/src/queries/Security/CWE-825/UnsafePointerEscapesClosure.qhelp

@@ -0,0 +1,21 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>Certain functions take a closure and pass a temporary pointer into it. If this pointer escapes from the closure and is used outside it, memory corruption may occur.</p>
+  </overview>
+
+  <recommendation>
+    <p>Do not use temporary pointers outside the closure they are passed to.</p>
+  </recommendation>
+
+  <example>
+    <p>In the first example below, the pointer is returned from the closure, potentially leading to memory corruption. In the second example, all work with the pointer is done inside the closure, and it is not returned.</p>
+    <sample src="UnsafePointerEscapesClosure.swift" />
+  </example>
+
+  <references>
+    <li><a href="https://developer.apple.com/documentation/swift/array/withunsafebytes(_:)">withUnsafeBytes</a></li>
+  </references>
+</qhelp>

swift/ql/src/queries/Security/CWE-825/UnsafePointerEscapesClosure.ql

@@ -0,0 +1,98 @@
+/**
+ * @name Unsafe pointer escapes closure
+ * @description Certain functions pass a low-level pointer to a closure. If this pointer outlives the closure, unpredictable results may occur.
+ * @kind path-problem
+ * @id swift/unsafe-pointer-escapes-closure
+ * @tags security
+ *       external/cwe/cwe-825
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import Flow::PathGraph
+
+module Config implements DataFlow::StateConfigSig {
+  class FlowState = Callable;
+
+  additional predicate isUnsafePointerCall(CallExpr call) {
+    call.getStaticTarget()
+    .hasName([
+        "withUnsafeBytes(_:)", "withCString(_:)", "withUnsafeMutableBytes(_:)",
+        "withContiguousMutableStorageIfAvailable(_:)", "withContiguousStorageIfAvailable(_:)",
+        "withUTF8(_:)", "withUnsafeBufferPointer(_:)", "withUnsafeMutableBufferPointer(_:)",
+        "withMemoryRebound(to:_:)", "withUnsafeTemporaryAllocation(of:capacity:_:)",
+        "withUnsafeCurrentTask(body:)", "withCheckedContinuation(function:_:)"
+      ])
+  }
+
+
+  additional predicate isUnsafePointerClosure(ClosureExpr expr) {
+    exists(CallExpr call |
+      isUnsafePointerCall(call) and
+      expr = call.getAnArgument().getExpr()
+    )
+  }
+
+  additional predicate isUnsafePointerFunction(Function f) {
+    exists(CallExpr call |
+      isUnsafePointerCall(call) and
+      f.getAnAccess() = call.getAnArgument().getExpr()
+    )
+  }
+  predicate isSource(DataFlow::Node node, FlowState state) {
+      (
+        isUnsafePointerClosure(state)
+        or
+        isUnsafePointerFunction(state)
+      ) and
+      state = node.(DataFlow::ParameterNode).getDeclaringFunction().getUnderlyingCallable()
+  }
+
+  predicate isSink(DataFlow::Node node, FlowState state) {
+    (
+      isUnsafePointerClosure(state)
+      or
+      isUnsafePointerFunction(state)
+    )
+    and
+    (
+      node.(DataFlow::InoutReturnNode).getParameter().getDeclaringFunction() = state
+      or
+      exists(ReturnStmt stmt |
+        node.asExpr() = stmt.getResult() and
+        stmt.getEnclosingCallable() = state
+      )
+    )
+  }
+
+  predicate isBarrier(DataFlow::Node node) { none() }
+
+  predicate isBarrierIn(DataFlow::Node node) { none() }
+
+  predicate isBarrierOut(DataFlow::Node node) { none() }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { 
+    node2.asExpr().convertsFrom(node1.asExpr())
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+  ) {
+    none()
+  }
+
+  int fieldFlowBranchLimit() { result = 2 }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
+    isSink(node, _) and
+    c = any(c)
+  }
+}
+
+module Flow = DataFlow::GlobalWithState<Config>;
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
+select sink, source, sink, "This unsafe parameter may escape its invocation"

swift/ql/src/queries/Security/CWE-825/UnsafePointerEscapesClosure.swift

@@ -0,0 +1,14 @@
+func bad() {
+    let ints = [1,2,3]
+    let bytes = ints.withUnsafeBytes{
+        return $0
+    }
+    print(bytes)
+}
+
+func good() {
+    let ints = [1,2,3]
+    let bytes = ints.withUnsafeBytes{
+        print($0)
+    }
+}

swift/ql/test/query-tests/Security/CWE-825/UnsafePointerEscapesClosure.expected

@@ -0,0 +1,27 @@
+edges
+| withUnsafeBytes.swift:9:25:9:25 | $0 | withUnsafeBytes.swift:10:16:10:16 | $0 |
+| withUnsafeBytes.swift:13:34:13:37 | p | withUnsafeBytes.swift:13:97:13:97 | p |
+| withUnsafeBytes.swift:15:34:15:37 | p | withUnsafeBytes.swift:15:109:15:109 | p |
+| withUnsafeBytes.swift:15:109:15:109 | p | withUnsafeBytes.swift:15:97:15:110 | call to id(pointer:) |
+| withUnsafeBytes.swift:15:109:15:109 | p | withUnsafeBytes.swift:38:12:38:21 | pointer |
+| withUnsafeBytes.swift:38:12:38:21 | pointer | withUnsafeBytes.swift:39:12:39:12 | pointer |
+| withUnsafeBytes.swift:38:12:38:21 | pointer | withUnsafeBytes.swift:39:12:39:12 | pointer |
+nodes
+| withUnsafeBytes.swift:9:25:9:25 | $0 | semmle.label | $0 |
+| withUnsafeBytes.swift:10:16:10:16 | $0 | semmle.label | $0 |
+| withUnsafeBytes.swift:13:34:13:37 | p | semmle.label | p |
+| withUnsafeBytes.swift:13:97:13:97 | p | semmle.label | p |
+| withUnsafeBytes.swift:15:34:15:37 | p | semmle.label | p |
+| withUnsafeBytes.swift:15:97:15:110 | call to id(pointer:) | semmle.label | call to id(pointer:) |
+| withUnsafeBytes.swift:15:109:15:109 | p | semmle.label | p |
+| withUnsafeBytes.swift:38:12:38:21 | pointer | semmle.label | pointer |
+| withUnsafeBytes.swift:38:12:38:21 | pointer | semmle.label | pointer |
+| withUnsafeBytes.swift:39:12:39:12 | pointer | semmle.label | pointer |
+| withUnsafeBytes.swift:39:12:39:12 | pointer | semmle.label | pointer |
+subpaths
+| withUnsafeBytes.swift:15:109:15:109 | p | withUnsafeBytes.swift:38:12:38:21 | pointer | withUnsafeBytes.swift:39:12:39:12 | pointer | withUnsafeBytes.swift:15:97:15:110 | call to id(pointer:) |
+#select
+| withUnsafeBytes.swift:10:16:10:16 | $0 | withUnsafeBytes.swift:9:25:9:25 | $0 | withUnsafeBytes.swift:10:16:10:16 | $0 | This unsafe parameter may escape its invocation |
+| withUnsafeBytes.swift:13:97:13:97 | p | withUnsafeBytes.swift:13:34:13:37 | p | withUnsafeBytes.swift:13:97:13:97 | p | This unsafe parameter may escape its invocation |
+| withUnsafeBytes.swift:15:97:15:110 | call to id(pointer:) | withUnsafeBytes.swift:15:34:15:37 | p | withUnsafeBytes.swift:15:97:15:110 | call to id(pointer:) | This unsafe parameter may escape its invocation |
+| withUnsafeBytes.swift:39:12:39:12 | pointer | withUnsafeBytes.swift:38:12:38:21 | pointer | withUnsafeBytes.swift:39:12:39:12 | pointer | This unsafe parameter may escape its invocation |

swift/ql/test/query-tests/Security/CWE-825/UnsafePointerEscapesClosure.qlref

@@ -0,0 +1 @@
+queries/Security/CWE-825/UnsafePointerEscapesClosure.ql

swift/ql/test/query-tests/Security/CWE-825/withUnsafeBytes.swift

@@ -0,0 +1,48 @@
+// stubs
+
+
+
+// tests
+
+func arrayTest() {
+    let ints = [1,2,3]
+    ints.withUnsafeBytes{ // BAD
+        return $0
+    }
+
+    print(ints.withUnsafeBytes({(p: UnsafeRawBufferPointer) -> UnsafeRawBufferPointer in return p})) // BAD
+
+    print(ints.withUnsafeBytes({(p: UnsafeRawBufferPointer) -> UnsafeRawBufferPointer in return id(pointer: p)})) // BAD
+
+    ints.withUnsafeBytes({(p: UnsafeRawBufferPointer) in print(p)}) // GOOD
+
+    var v = PointerHolder()
+    ints.withUnsafeBytes({(p: UnsafeRawBufferPointer) in
+      v.field = p
+      return 1
+    }) // BAD
+    print(v.field)
+
+    ints.withUnsafeBytes(myPrint) // GOOD
+
+    myPrint(p: ints.withUnsafeBytes(id)) // BAD
+
+    var v2: UnsafeRawBufferPointer? = nil
+    ints.withUnsafeBytes({(p: UnsafeRawBufferPointer) in
+        v2 = p
+        return 1
+    }) // BAD
+    print(v2)
+}
+
+func id<T>(pointer: T) -> T {
+    return pointer
+}
+
+struct PointerHolder {
+    var field: UnsafeRawBufferPointer?
+}
+
+func myPrint(p: UnsafeRawBufferPointer) {
+    print(p)
+}