Java: Flag "intentionally" unsafe methods in tests.

intrigus · intrigus · commit 484533c659a3 · 2021-06-25T16:47:23.000+02:00
Previously intentionally unsafe methods such as `disableCertificate`
would be ignored by this query. But now they will also be flagged
as it is hard to guess intentions...
Adjust the tests to account for this change.
diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager.expected
@@ -1,10 +1,14 @@
 edges
-| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager |
-| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:49:24:49:35 | trustManager |
+| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager |
+| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:51:24:51:35 | trustManager |
+| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager |
 nodes
-| InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
-| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | semmle.label | trustManager |
-| InsecureTrustManagerTest.java:48:56:48:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
-| InsecureTrustManagerTest.java:49:24:49:35 | trustManager | semmle.label | trustManager |
+| InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
+| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | semmle.label | trustManager |
+| InsecureTrustManagerTest.java:50:56:50:81 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
+| InsecureTrustManagerTest.java:51:24:51:35 | trustManager | semmle.label | trustManager |
+| InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | semmle.label | new InsecureTrustManager(...) : InsecureTrustManager |
+| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | semmle.label | trustManager |
 #select
-| InsecureTrustManagerTest.java:41:23:41:34 | trustManager | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:41:23:41:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:40:55:40:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:20:23:20:42 | InsecureTrustManager | here |
+| InsecureTrustManagerTest.java:43:23:43:34 | trustManager | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:43:23:43:34 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:42:55:42:80 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here |
+| InsecureTrustManagerTest.java:92:22:92:33 | trustManager | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | InsecureTrustManagerTest.java:92:22:92:33 | trustManager | $@ that is defined $@ and trusts any certificate, is used here. | InsecureTrustManagerTest.java:91:54:91:79 | new InsecureTrustManager(...) : InsecureTrustManager | This trustmanager | InsecureTrustManagerTest.java:22:23:22:42 | InsecureTrustManager | here |
diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManagerTest.java
@@ -89,9 +89,9 @@ public static void main(String[] args) throws Exception {
 	private static void disableTrustManager() throws NoSuchAlgorithmException, KeyManagementException {
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };
-		context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the
+		context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the
 												// certificate
-		// chain, allowing any certificate. BUT it is the method name suggests that this
-		// is intentional.
+		// chain, allowing any certificate. The method name suggests that this may be
+		// intentional, but we flag it anyway.
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -89,9 +89,9 @@ public static void main(String[] args) throws Exception {`
`89`	`89`	`private static void disableTrustManager() throws NoSuchAlgorithmException, KeyManagementException {`
`90`	`90`	`SSLContext context = SSLContext.getInstance("TLS");`
`91`	`91`	`TrustManager[] trustManager = new TrustManager[] { new InsecureTrustManager() };`
`92`		- context.init(null, trustManager, null); // GOOD: Uses a `TrustManager` that does not verify the
	`92`	+ context.init(null, trustManager, null); // BAD: Uses a `TrustManager` that does not verify the
`93`	`93`	`// certificate`
`94`		`- // chain, allowing any certificate. BUT it is the method name suggests that this`
`95`		`- // is intentional.`
	`94`	`+ // chain, allowing any certificate. The method name suggests that this may be`
	`95`	`+ // intentional, but we flag it anyway.`
`96`	`96`	`}`
`97`	`97`	`}`