Apply suggestions from code review

Co-authored-by: Rasmus Wriedt Larsen <[email protected]>

Author: yoff

python/ql/src/Security/CWE-327/FluentApiModel.qll

@@ -3,7 +3,8 @@ import TlsLibraryModel
 
 /**
  * Configuration to determine the state of a context being used to create
- * a conection.
+ * a conection. There is one configuration for each pair of `TlsLibrary` and `ProtocolVersion`,
+ * such that a single configuration only tracks contexts where a specific `ProtocolVersion` is allowed.
  *
  * The state is in terms of whether a specific protocol is allowed. This is
  * either true or false when the context is created and can then be modified
@@ -72,20 +73,18 @@ predicate unsafe_connection_creation_with_context(
   boolean specific
 ) {
   // Connection created from a context allowing `insecure_version`.
-  exists(InsecureContextConfiguration c, ProtocolUnrestriction co |
-    c.hasFlow(co, connectionCreation)
+  exists(InsecureContextConfiguration c |
+    c.hasFlow(contextOrigin, connectionCreation)
   |
     insecure_version = c.getTrackedVersion() and
-    contextOrigin = co and
+    contextOrigin instanceof ProtocolUnrestriction and
     specific = false
   )
   or
   // Connection created from a context specifying `insecure_version`.
-  exists(TlsLibrary l, DataFlow::CfgNode cc |
-    cc = l.insecure_connection_creation(insecure_version)
-  |
-    connectionCreation = cc and
-    contextOrigin = cc and
+  exists(TlsLibrary l |
+    connectionCreation = l.insecure_connection_creation(insecure_version) and
+    contextOrigin = connectionCreation and
     specific = true
   )
 }
@@ -105,7 +104,6 @@ predicate unsafe_connection_creation_without_context(
 
 /** Holds if `contextCreation` is creating a context ties to a specific insecure version. */
 predicate unsafe_context_creation(DataFlow::CallCfgNode contextCreation, string insecure_version) {
-  exists(TlsLibrary l, ContextCreation cc | cc = l.insecure_context_creation(insecure_version) |
-    contextCreation = cc
-  )
+  contextCreation instanceof ContextCreation and
+  exists(TlsLibrary l | contextCreation = l.insecure_context_creation(insecure_version))
 }

python/ql/src/Security/CWE-327/Ssl.qll

@@ -23,30 +23,17 @@ class SSLDefaultContextCreation extends ContextCreation {
 }
 
 /** Gets a reference to an `ssl.Context` instance. */
-private DataFlow::LocalSourceNode sslContextInstance(DataFlow::TypeTracker t) {
-  t.start() and
-  result = API::moduleImport("ssl").getMember(["SSLContext", "create_default_context"]).getACall()
-  or
-  exists(DataFlow::TypeTracker t2 | result = sslContextInstance(t2).track(t2, t))
-}
-
-/** Gets a reference to an `ssl.Context` instance. */
-DataFlow::Node sslContextInstance() {
-  sslContextInstance(DataFlow::TypeTracker::end()).flowsTo(result)
+API::Node sslContextInstance() {
+  result = API::moduleImport("ssl").getMember(["SSLContext", "create_default_context"]).getReturn()
 }
 
-class WrapSocketCall extends ConnectionCreation {
-  override CallNode node;
-
+class WrapSocketCall extends ConnectionCreation, DataFlow::CallCfgNode {
   WrapSocketCall() {
-    exists(DataFlow::AttrRead call | node.getFunction() = call.asCfgNode() |
-      call.getAttributeName() = "wrap_socket" and
-      call.getObject() = sslContextInstance()
-    )
+    this = sslContextInstance().getMember("wrap_socket").getACall()
   }
 
   override DataFlow::CfgNode getContext() {
-    result.getNode() = node.getFunction().(AttrNode).getObject()
+    result = this.getFunction().(DataFlow::AttrRead).getObject()
   }
 }
 

python/ql/src/Security/CWE-327/TlsLibraryModel.qll

@@ -84,7 +84,7 @@ abstract class UnspecificContextCreation extends ContextCreation, ProtocolUnrest
   }
 }
 
-/** A model of a TLS library. */
+/** A model of a SSL/TLS library. */
 abstract class TlsLibrary extends string {
   TlsLibrary() { this in ["ssl", "pyOpenSSL"] }