Swift: Add default implicit read steps when selecting PostUpdateNodes as sinks.

Author: MathiasVP

swift/ql/lib/codeql/swift/dataflow/internal/TaintTrackingPublic.qll

@@ -26,4 +26,14 @@ predicate localTaintStep = localTaintStepCached/2;
  * of `c` at sinks and inputs to additional taint steps.
  */
 bindingset[node]
-predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet c) { none() }
+predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+  // If a `PostUpdateNode` is specified as a sink, there's (almost) always a store step preceding it.
+  // So when the node is a `PostUpdateNode` we allow any sequence of implicit read steps of an appropriate
+  // type to make sure we arrive at the sink with an empty access path.
+  exists(NominalTypeDecl d, Decl cx |
+    node.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getType() =
+      d.getType().getABaseType*() and
+    cx.asNominalTypeDecl() = d and
+    cs.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
+  )
+}

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

@@ -22,17 +22,6 @@ module InsecureTlsConfig implements DataFlow::ConfigSig {
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
     any(InsecureTlsExtensionsAdditionalTaintStep s).step(nodeFrom, nodeTo)
   }
-
-  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
-    // flow out from fields of an `URLSessionConfiguration` at the sink,
-    // for example in `sessionConfig.tlsMaximumSupportedProtocolVersion = tls_protocol_version_t.TLSv10`.
-    isSink(node) and
-    exists(NominalTypeDecl d, Decl cx |
-      d.getType().getABaseType*().getUnderlyingType().getName() = "URLSessionConfiguration" and
-      cx.asNominalTypeDecl() = d and
-      c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
-    )
-  }
 }
 
 module InsecureTlsFlow = TaintTracking::Global<InsecureTlsConfig>;

swift/ql/test/query-tests/Security/CWE-757/InsecureTLS.expected

@@ -49,10 +49,6 @@ edges
 | InsecureTLS.swift:181:53:181:76 | .TLSv10 :  | InsecureTLS.swift:19:7:19:7 | value :  |
 | InsecureTLS.swift:181:53:181:76 | .TLSv10 :  | InsecureTLS.swift:181:3:181:9 | [post] getter for .config |
 | InsecureTLS.swift:181:53:181:76 | .TLSv10 :  | InsecureTLS.swift:181:3:181:9 | [post] getter for .config [tlsMinimumSupportedProtocolVersion] :  |
-| InsecureTLS.swift:185:20:185:36 | withMinVersion :  | InsecureTLS.swift:187:42:187:42 | withMinVersion :  |
-| InsecureTLS.swift:187:5:187:5 | [post] self [tlsMinimumSupportedProtocolVersion] :  | InsecureTLS.swift:187:5:187:5 | [post] self |
-| InsecureTLS.swift:187:42:187:42 | withMinVersion :  | InsecureTLS.swift:187:5:187:5 | [post] self [tlsMinimumSupportedProtocolVersion] :  |
-| InsecureTLS.swift:193:51:193:74 | .TLSv10 :  | InsecureTLS.swift:185:20:185:36 | withMinVersion :  |
 | file://:0:0:0:0 | [post] self [tlsMaximumSupportedProtocolVersion] :  | file://:0:0:0:0 | [post] self |
 | file://:0:0:0:0 | [post] self [tlsMaximumSupportedProtocolVersion] :  | file://:0:0:0:0 | [post] self :  |
 | file://:0:0:0:0 | [post] self [tlsMaximumSupportedProtocol] :  | file://:0:0:0:0 | [post] self |
@@ -107,11 +103,6 @@ nodes
 | InsecureTLS.swift:181:3:181:9 | [post] getter for .config | semmle.label | [post] getter for .config |
 | InsecureTLS.swift:181:3:181:9 | [post] getter for .config [tlsMinimumSupportedProtocolVersion] :  | semmle.label | [post] getter for .config [tlsMinimumSupportedProtocolVersion] :  |
 | InsecureTLS.swift:181:53:181:76 | .TLSv10 :  | semmle.label | .TLSv10 :  |
-| InsecureTLS.swift:185:20:185:36 | withMinVersion :  | semmle.label | withMinVersion :  |
-| InsecureTLS.swift:187:5:187:5 | [post] self | semmle.label | [post] self |
-| InsecureTLS.swift:187:5:187:5 | [post] self [tlsMinimumSupportedProtocolVersion] :  | semmle.label | [post] self [tlsMinimumSupportedProtocolVersion] :  |
-| InsecureTLS.swift:187:42:187:42 | withMinVersion :  | semmle.label | withMinVersion :  |
-| InsecureTLS.swift:193:51:193:74 | .TLSv10 :  | semmle.label | .TLSv10 :  |
 | file://:0:0:0:0 | .TLSVersion :  | semmle.label | .TLSVersion :  |
 | file://:0:0:0:0 | [post] self | semmle.label | [post] self |
 | file://:0:0:0:0 | [post] self | semmle.label | [post] self |
@@ -163,7 +154,6 @@ subpaths
 | InsecureTLS.swift:122:3:122:3 | [post] config | InsecureTLS.swift:127:25:127:48 | .TLSv11 :  | InsecureTLS.swift:122:3:122:3 | [post] config | This TLS configuration is insecure. |
 | InsecureTLS.swift:165:3:165:3 | [post] config | InsecureTLS.swift:163:20:163:43 | .TLSv10 :  | InsecureTLS.swift:165:3:165:3 | [post] config | This TLS configuration is insecure. |
 | InsecureTLS.swift:181:3:181:9 | [post] getter for .config | InsecureTLS.swift:181:53:181:76 | .TLSv10 :  | InsecureTLS.swift:181:3:181:9 | [post] getter for .config | This TLS configuration is insecure. |
-| InsecureTLS.swift:187:5:187:5 | [post] self | InsecureTLS.swift:193:51:193:74 | .TLSv10 :  | InsecureTLS.swift:187:5:187:5 | [post] self | This TLS configuration is insecure. |
 | file://:0:0:0:0 | [post] self | InsecureTLS.swift:40:47:40:70 | .TLSv10 :  | file://:0:0:0:0 | [post] self | This TLS configuration is insecure. |
 | file://:0:0:0:0 | [post] self | InsecureTLS.swift:45:47:45:70 | .TLSv11 :  | file://:0:0:0:0 | [post] self | This TLS configuration is insecure. |
 | file://:0:0:0:0 | [post] self | InsecureTLS.swift:57:47:57:70 | .TLSv10 :  | file://:0:0:0:0 | [post] self | This TLS configuration is insecure. |