Change InsecureTrustManagerConfiguration to DataFlow

atorralba · atorralba · commit 967308fbfda5 · 2022-01-20T10:24:47.000+01:00
diff --git a/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll b/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll
@@ -2,19 +2,24 @@
 
 import java
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.Encryption
 import semmle.code.java.security.InsecureTrustManager
 
 /**
  * A configuration to model the flow of an insecure `TrustManager`
  * to the initialization of an SSL context.
  */
-class InsecureTrustManagerConfiguration extends TaintTracking::Configuration {
+class InsecureTrustManagerConfiguration extends DataFlow::Configuration {
   InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" }
 
   override predicate isSource(DataFlow::Node source) {
     source instanceof InsecureTrustManagerSource
   }
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureTrustManagerSink }
+
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+    (this.isSink(node) or this.isAdditionalFlowStep(node, _)) and
+    node.getType() instanceof Array and
+    c instanceof DataFlow::ArrayContent
+  }
 }
diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.java b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.java
@@ -121,7 +121,7 @@ private static void directInsecureTrustManagerCall()
 			throws NoSuchAlgorithmException, KeyManagementException {
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 	}
 
 	private static void namedVariableFlagDirectInsecureTrustManagerCall()
@@ -145,7 +145,7 @@ private static void noNamedVariableFlagDirectInsecureTrustManagerCall()
 		if (SOME_NAME_THAT_IS_NOT_A_FLAG_NAME) {
 			SSLContext context = SSLContext.getInstance("TLS");
 			TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-			context.init(null, trustManager, null); // $ hasTaintFlow
+			context.init(null, trustManager, null); // $ hasValueFlow
 		}
 	}
 
@@ -177,7 +177,7 @@ private static void noStringLiteralFlagDirectInsecureTrustManagerCall()
 		if (Boolean.parseBoolean(System.getProperty("SOME_NAME_THAT_IS_NOT_A_FLAG_NAME"))) {
 			SSLContext context = SSLContext.getInstance("TLS");
 			TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-			context.init(null, trustManager, null); // $ hasTaintFlow
+			context.init(null, trustManager, null); // $ hasValueFlow
 		}
 	}
 
@@ -209,7 +209,7 @@ private static void noMethodAccessFlagDirectInsecureTrustManagerCall()
 		if (is42TheAnswerForEverything()) {
 			SSLContext context = SSLContext.getInstance("TLS");
 			TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-			context.init(null, trustManager, null); // $ hasTaintFlow
+			context.init(null, trustManager, null); // $ hasValueFlow
 		}
 	}
 
@@ -226,7 +226,7 @@ private static void isEqualsIgnoreCaseDirectInsecureTrustManagerCall()
 		if (schemaFromHttpRequest.equalsIgnoreCase("https")) {
 			SSLContext context = SSLContext.getInstance("TLS");
 			TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-			context.init(null, trustManager, null); // $ hasTaintFlow
+			context.init(null, trustManager, null); // $ hasValueFlow
 		}
 	}
 
@@ -244,7 +244,7 @@ private static void noIsEqualsIgnoreCaseDirectInsecureTrustManagerCall()
 		if (!schemaFromHttpRequest.equalsIgnoreCase("https")) {
 			SSLContext context = SSLContext.getInstance("TLS");
 			TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-			context.init(null, trustManager, null); // $ hasTaintFlow
+			context.init(null, trustManager, null); // $ hasValueFlow
 		}
 	}
 
@@ -264,7 +264,7 @@ private static void namedVariableFlagNOTGuardingDirectInsecureTrustManagerCall()
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -276,7 +276,7 @@ private static void noNamedVariableFlagNOTGuardingDirectInsecureTrustManagerCall
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -288,7 +288,7 @@ private static void stringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall()
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -300,7 +300,7 @@ private static void noStringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -312,7 +312,7 @@ private static void methodAccessFlagNOTGuardingDirectInsecureTrustManagerCall()
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -324,7 +324,7 @@ private static void noMethodAccessFlagNOTGuardingDirectInsecureTrustManagerCall(
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 	}
 
 	private static void isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall()
@@ -336,7 +336,7 @@ private static void isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall(
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
@@ -349,14 +349,14 @@ private static void noIsEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCal
 
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 
 	}
 
 	private static void disableTrustManager()
 			throws NoSuchAlgorithmException, KeyManagementException {
 		SSLContext context = SSLContext.getInstance("TLS");
 		TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};
-		context.init(null, trustManager, null); // $ hasTaintFlow
+		context.init(null, trustManager, null); // $ hasValueFlow
 	}
 }
diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql
@@ -3,9 +3,7 @@ import semmle.code.java.security.InsecureTrustManagerQuery
 import TestUtilities.InlineFlowTest
 
 class InsecureTrustManagerTest extends InlineFlowTest {
-  override DataFlow::Configuration getValueFlowConfig() { none() }
-
-  override TaintTracking::Configuration getTaintFlowConfig() {
+  override DataFlow::Configuration getValueFlowConfig() {
     result = any(InsecureTrustManagerConfiguration c)
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ private static void directInsecureTrustManagerCall()`
`121`	`121`	`throws NoSuchAlgorithmException, KeyManagementException {`
`122`	`122`	`SSLContext context = SSLContext.getInstance("TLS");`
`123`	`123`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`124`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`124`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`125`	`125`	`}`
`126`	`126`
`127`	`127`	`private static void namedVariableFlagDirectInsecureTrustManagerCall()`
`@@ -145,7 +145,7 @@ private static void noNamedVariableFlagDirectInsecureTrustManagerCall()`
`145`	`145`	`if (SOME_NAME_THAT_IS_NOT_A_FLAG_NAME) {`
`146`	`146`	`SSLContext context = SSLContext.getInstance("TLS");`
`147`	`147`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`148`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`148`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`149`	`149`	`}`
`150`	`150`	`}`
`151`	`151`
`@@ -177,7 +177,7 @@ private static void noStringLiteralFlagDirectInsecureTrustManagerCall()`
`177`	`177`	`if (Boolean.parseBoolean(System.getProperty("SOME_NAME_THAT_IS_NOT_A_FLAG_NAME"))) {`
`178`	`178`	`SSLContext context = SSLContext.getInstance("TLS");`
`179`	`179`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`180`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`180`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`181`	`181`	`}`
`182`	`182`	`}`
`183`	`183`
`@@ -209,7 +209,7 @@ private static void noMethodAccessFlagDirectInsecureTrustManagerCall()`
`209`	`209`	`if (is42TheAnswerForEverything()) {`
`210`	`210`	`SSLContext context = SSLContext.getInstance("TLS");`
`211`	`211`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`212`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`212`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`213`	`213`	`}`
`214`	`214`	`}`
`215`	`215`
`@@ -226,7 +226,7 @@ private static void isEqualsIgnoreCaseDirectInsecureTrustManagerCall()`
`226`	`226`	`if (schemaFromHttpRequest.equalsIgnoreCase("https")) {`
`227`	`227`	`SSLContext context = SSLContext.getInstance("TLS");`
`228`	`228`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`229`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`229`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`230`	`230`	`}`
`231`	`231`	`}`
`232`	`232`
`@@ -244,7 +244,7 @@ private static void noIsEqualsIgnoreCaseDirectInsecureTrustManagerCall()`
`244`	`244`	`if (!schemaFromHttpRequest.equalsIgnoreCase("https")) {`
`245`	`245`	`SSLContext context = SSLContext.getInstance("TLS");`
`246`	`246`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`247`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`247`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`248`	`248`	`}`
`249`	`249`	`}`
`250`	`250`
`@@ -264,7 +264,7 @@ private static void namedVariableFlagNOTGuardingDirectInsecureTrustManagerCall()`
`264`	`264`
`265`	`265`	`SSLContext context = SSLContext.getInstance("TLS");`
`266`	`266`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`267`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`267`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`268`	`268`
`269`	`269`	`}`
`270`	`270`
`@@ -276,7 +276,7 @@ private static void noNamedVariableFlagNOTGuardingDirectInsecureTrustManagerCall`
`276`	`276`
`277`	`277`	`SSLContext context = SSLContext.getInstance("TLS");`
`278`	`278`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`279`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`279`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`280`	`280`
`281`	`281`	`}`
`282`	`282`
`@@ -288,7 +288,7 @@ private static void stringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall()`
`288`	`288`
`289`	`289`	`SSLContext context = SSLContext.getInstance("TLS");`
`290`	`290`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`291`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`291`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`292`	`292`
`293`	`293`	`}`
`294`	`294`
`@@ -300,7 +300,7 @@ private static void noStringLiteralFlagNOTGuardingDirectInsecureTrustManagerCall`
`300`	`300`
`301`	`301`	`SSLContext context = SSLContext.getInstance("TLS");`
`302`	`302`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`303`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`303`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`304`	`304`
`305`	`305`	`}`
`306`	`306`
`@@ -312,7 +312,7 @@ private static void methodAccessFlagNOTGuardingDirectInsecureTrustManagerCall()`
`312`	`312`
`313`	`313`	`SSLContext context = SSLContext.getInstance("TLS");`
`314`	`314`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`315`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`315`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`316`	`316`
`317`	`317`	`}`
`318`	`318`
`@@ -324,7 +324,7 @@ private static void noMethodAccessFlagNOTGuardingDirectInsecureTrustManagerCall(`
`324`	`324`
`325`	`325`	`SSLContext context = SSLContext.getInstance("TLS");`
`326`	`326`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`327`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`327`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`328`	`328`	`}`
`329`	`329`
`330`	`330`	`private static void isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall()`
`@@ -336,7 +336,7 @@ private static void isEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCall(`
`336`	`336`
`337`	`337`	`SSLContext context = SSLContext.getInstance("TLS");`
`338`	`338`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`339`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`339`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`340`	`340`
`341`	`341`	`}`
`342`	`342`
`@@ -349,14 +349,14 @@ private static void noIsEqualsIgnoreCaseNOTGuardingDirectInsecureTrustManagerCal`
`349`	`349`
`350`	`350`	`SSLContext context = SSLContext.getInstance("TLS");`
`351`	`351`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`352`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`352`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`353`	`353`
`354`	`354`	`}`
`355`	`355`
`356`	`356`	`private static void disableTrustManager()`
`357`	`357`	`throws NoSuchAlgorithmException, KeyManagementException {`
`358`	`358`	`SSLContext context = SSLContext.getInstance("TLS");`
`359`	`359`	`TrustManager[] trustManager = new TrustManager[] {new InsecureTrustManager()};`
`360`		`- context.init(null, trustManager, null); // $ hasTaintFlow`
	`360`	`+ context.init(null, trustManager, null); // $ hasValueFlow`
`361`	`361`	`}`
`362`	`362`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,9 +3,7 @@ import semmle.code.java.security.InsecureTrustManagerQuery`
`3`	`3`	`import TestUtilities.InlineFlowTest`
`4`	`4`
`5`	`5`	`class InsecureTrustManagerTest extends InlineFlowTest {`
`6`		`- override DataFlow::Configuration getValueFlowConfig() { none() }`
`7`		`-`
`8`		`- override TaintTracking::Configuration getTaintFlowConfig() {`
	`6`	`+ override DataFlow::Configuration getValueFlowConfig() {`
`9`	`7`	`result = any(InsecureTrustManagerConfiguration c)`
`10`	`8`	`}`
`11`	`9`	`}`