Fix assumption regarding when an SSLSocket does the TLS handhsake

Author: atorralba

java/ql/lib/semmle/code/java/frameworks/Networking.qll

@@ -47,6 +47,14 @@ class SocketGetInputStreamMethod extends Method {
   }
 }
 
+class SocketGetOutputStreamMethod extends Method {
+  SocketGetOutputStreamMethod() {
+    this.getDeclaringType() instanceof TypeSocket and
+    this.hasName("getOutputStream") and
+    this.hasNoParameters()
+  }
+}
+
 /** A method or constructor call that returns a new `URI`. */
 class UriCreation extends Call {
   UriCreation() {
@@ -152,7 +160,7 @@ class UrlOpenConnectionMethod extends Method {
 class CreateSocketMethod extends Method {
   CreateSocketMethod() {
     this.hasName("createSocket") and
-    this.getDeclaringType() instanceof TypeSocketFactory
+    this.getDeclaringType().getASupertype*() instanceof TypeSocketFactory
   }
 }
 

java/ql/lib/semmle/code/java/security/UnsafeCertTrust.qll

@@ -13,8 +13,14 @@ private import semmle.code.java.dataflow.DataFlow2
  */
 class SslConnectionInit extends DataFlow::Node {
   SslConnectionInit() {
-    this.asExpr().(MethodAccess).getMethod() instanceof CreateSslEngineMethod or
-    this.asExpr().(MethodAccess).getMethod() instanceof CreateSocketMethod
+    exists(MethodAccess ma, Method m |
+      this.asExpr() = ma and
+      ma.getMethod() = m
+    |
+      m instanceof CreateSslEngineMethod
+      or
+      m instanceof CreateSocketMethod and isSslSocket(ma)
+    )
   }
 }
 
@@ -29,21 +35,11 @@ class SslConnectionCreation extends DataFlow::Node {
       m instanceof BeginHandshakeMethod or
       m instanceof SslWrapMethod or
       m instanceof SslUnwrapMethod or
-      m instanceof SocketConnectMethod
+      m instanceof SocketGetOutputStreamMethod
     |
       ma.getMethod() = m and
       this.asExpr() = ma.getQualifier()
     )
-    or
-    // calls to SocketFactory.createSocket with parameters immediately create the connection
-    exists(MethodAccess ma, Method m |
-      ma.getMethod() = m and
-      m instanceof CreateSocketMethod and
-      m.getNumberOfParameters() > 0 and
-      isSslSocket(ma)
-    |
-      this.asExpr() = ma
-    )
   }
 }
 

java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.java

@@ -1,4 +1,3 @@
-import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.nio.ByteBuffer;
 import javax.net.SocketFactory;
@@ -25,9 +24,6 @@ public void testSSLEngineEndpointIdSetNull() throws Exception {
 		sslEngine.unwrap(null, null, 0, 0); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the endpoint identification of SSL engine is set to null
-	 */
 	public void testSSLEngineEndpointIdSetEmpty() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLEngine sslEngine = sslContext.createSSLEngine();
@@ -39,9 +35,6 @@ public void testSSLEngineEndpointIdSetEmpty() throws Exception {
 		sslEngine.unwrap(null, null, 0, 0); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the endpoint identification of SSL engine is set to HTTPS
-	 */
 	public void testSSLEngineEndpointIdSafe() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLEngine sslEngine = sslContext.createSSLEngine();
@@ -53,9 +46,6 @@ public void testSSLEngineEndpointIdSafe() throws Exception {
 		sslEngine.unwrap(null, null, 0, 0); // Safe
 	}
 
-	/**
-	 * Test the endpoint identification of SSL engine is set to HTTPS
-	 */
 	public void testSSLEngineInServerMode() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLEngine sslEngine = sslContext.createSSLEngine();
@@ -65,95 +55,64 @@ public void testSSLEngineInServerMode() throws Exception {
 		sslEngine.unwrap(null, null, 0, 0); // Safe
 	}
 
-	/**
-	 * Test the endpoint identification of SSL socket is not set
-	 */
-	public void testSSLSocketImmediatelyConnects() throws Exception {
-		SSLContext sslContext = SSLContext.getInstance("TLS");
-		SocketFactory socketFactory = sslContext.getSocketFactory();
-		SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443); // $hasUnsafeCertTrust
-	}
-
-	/**
-	 * Test the endpoint identification of SSL socket is not set
-	 */
 	public void testSSLSocketEndpointIdNotSet() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLSocketFactory socketFactory = sslContext.getSocketFactory();
 		SSLSocket socket = (SSLSocket) socketFactory.createSocket();
-		socket.connect(new InetSocketAddress("www.example.com", 443)); // $hasUnsafeCertTrust
+		socket.getOutputStream(); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the endpoint identification of SSL socket is set to null
-	 */
 	public void testSSLSocketEndpointIdSetNull() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLSocketFactory socketFactory = sslContext.getSocketFactory();
 		SSLSocket socket = (SSLSocket) socketFactory.createSocket();
 		SSLParameters sslParameters = socket.getSSLParameters();
 		sslParameters.setEndpointIdentificationAlgorithm(null);
 		socket.setSSLParameters(sslParameters);
-		socket.connect(new InetSocketAddress("www.example.com", 443)); // $hasUnsafeCertTrust
+		socket.getOutputStream(); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the endpoint identification of SSL socket is set to empty
-	 */
 	public void testSSLSocketEndpointIdSetEmpty() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLSocketFactory socketFactory = sslContext.getSocketFactory();
 		SSLSocket socket = (SSLSocket) socketFactory.createSocket();
 		SSLParameters sslParameters = socket.getSSLParameters();
 		sslParameters.setEndpointIdentificationAlgorithm("");
 		socket.setSSLParameters(sslParameters);
-		socket.connect(new InetSocketAddress("www.example.com", 443)); // $hasUnsafeCertTrust
+		socket.getOutputStream(); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the endpoint identification of SSL socket is not set
-	 */
 	public void testSSLSocketEndpointIdAfterConnecting() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLSocketFactory socketFactory = sslContext.getSocketFactory();
-		SSLSocket socket = (SSLSocket) socketFactory.createSocket("www.example.com", 443); // $hasUnsafeCertTrust
+		SSLSocket socket = (SSLSocket) socketFactory.createSocket();
+		socket.getOutputStream(); // $hasUnsafeCertTrust
 		SSLParameters sslParameters = socket.getSSLParameters();
 		sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
 		socket.setSSLParameters(sslParameters);
 	}
 
-	/**
-	 * Test the endpoint identification of SSL socket is not set
-	 */
 	public void testSSLSocketEndpointIdSafe() throws Exception {
 		SSLContext sslContext = SSLContext.getInstance("TLS");
 		SSLSocketFactory socketFactory = sslContext.getSocketFactory();
 		SSLSocket socket = (SSLSocket) socketFactory.createSocket();
 		SSLParameters sslParameters = socket.getSSLParameters();
 		sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
 		socket.setSSLParameters(sslParameters);
-		socket.connect(new InetSocketAddress("www.example.com", 443)); // Safe
+		socket.getOutputStream(); // Safe
 	}
 
-	/**
-	 * Test the endpoint identification of regular socket is not set
-	 */
 	public void testSocketEndpointIdNotSet() throws Exception {
 		SocketFactory socketFactory = SocketFactory.getDefault();
-		Socket socket = socketFactory.createSocket("www.example.com", 80); // Safe
+		Socket socket = socketFactory.createSocket("www.example.com", 80);
+		socket.getOutputStream(); // Safe
 	}
 
-	/**
-	 * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
-	 */
 	public void testRabbitMQFactoryEnableHostnameVerificationNotSet() throws Exception {
 		ConnectionFactory connectionFactory = new ConnectionFactory();
 		connectionFactory.useSslProtocol(); // $hasUnsafeCertTrust
 	}
 
-	/**
-	 * Test the enableHostnameVerification of RabbitMQConnectionFactory is not set
-	 */
 	public void testRabbitMQFactorySafe() throws Exception {
 		ConnectionFactory connectionFactory = new ConnectionFactory();
 		connectionFactory.useSslProtocol(); // Safe