make one config for asymm with flow states; seems to work...

Author: Jami Cogswell

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

@@ -2,6 +2,84 @@
 
 import semmle.code.java.security.Encryption
 import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.TaintTracking
+
+//import semmle.code.java.dataflow.internal.DataFlowImplCommonPublic
+//import semmle.code.java.dataflow.FlowSources
+//import semmle.code.java.dataflow.internal.DataFlowNodes
+/**
+ * An Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
+ */
+class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration {
+  AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" }
+
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+    //state instanceof DataFlow::FlowStateEmpty and
+    source.asExpr().(IntegerLiteral).getIntValue() < 2048 and state = "2048"
+    or
+    source.asExpr().(IntegerLiteral).getIntValue() < 256 and state = "256"
+    or
+    getECKeySize(source.asExpr().(StringLiteral).getValue()) < 256 and state = "256" // need this for the cases when the key size is embedded in the curve name.
+  }
+
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+    exists(MethodAccess ma, JavaSecurityKeyPairGenerator jpg |
+      ma.getMethod() instanceof KeyPairGeneratorInitMethod and
+      (
+        jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and
+        DataFlow::localExprFlow(jpg, ma.getQualifier()) and
+        sink.asExpr() = ma.getArgument(0) and
+        //ma.getArgument(0).(LocalSourceNode).flowsTo(sink) and
+        //ma.getArgument(0).(CompileTimeConstantExpr).getIntValue() < 2048 and
+        state = "2048"
+      )
+      or
+      jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and
+      DataFlow::localExprFlow(jpg, ma.getQualifier()) and
+      sink.asExpr() = ma.getArgument(0) and
+      //ma.getArgument(0).(CompileTimeConstantExpr).getIntValue() < 256 and
+      state = "256"
+    )
+    or
+    // TODO: combine below three for less duplicated code
+    exists(ClassInstanceExpr rsaKeyGenParamSpec |
+      rsaKeyGenParamSpec.getConstructedType() instanceof RsaKeyGenParameterSpec and
+      sink.asExpr() = rsaKeyGenParamSpec.getArgument(0) and
+      state = "2048"
+    )
+    or
+    exists(ClassInstanceExpr dsaGenParamSpec |
+      dsaGenParamSpec.getConstructedType() instanceof DsaGenParameterSpec and
+      sink.asExpr() = dsaGenParamSpec.getArgument(0) and
+      state = "2048"
+    )
+    or
+    exists(ClassInstanceExpr dhGenParamSpec |
+      dhGenParamSpec.getConstructedType() instanceof DhGenParameterSpec and
+      sink.asExpr() = dhGenParamSpec.getArgument(0) and
+      state = "2048"
+    )
+    or
+    exists(ClassInstanceExpr ecGenParamSpec |
+      ecGenParamSpec.getConstructedType() instanceof EcGenParameterSpec and
+      sink.asExpr() = ecGenParamSpec.getArgument(0) and
+      state = "256"
+    )
+  }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    exists(IntegerLiteral intLiteral |
+      state1 = "" and
+      state2 = intLiteral.toString() and
+      node1.asExpr() = intLiteral and
+      node2.asExpr() = intLiteral
+      //intLiteral.toString().toInt() = 64 // test viability of this craziness
+    )
+  }
+}
 
 /**
  * An Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.

java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql

@@ -17,7 +17,9 @@ import DataFlow::PathGraph
 
 from DataFlow::PathNode source, DataFlow::PathNode sink
 where
-  exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or
-  exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or
+  exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
+  or
+  //   exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or
+  //   exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or
   exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
 select sink.getNode(), source, sink, "This $@ is too small.", source.getNode(), "key size"

java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql

@@ -1,6 +1,7 @@
 import java
 import TestUtilities.InlineExpectationsTest
 import semmle.code.java.security.InsufficientKeySizeQuery
+import DataFlow::PathGraph
 
 class InsufficientKeySizeTest extends InlineExpectationsTest {
   InsufficientKeySizeTest() { this = "InsufficientKeySize" }
@@ -9,13 +10,15 @@ class InsufficientKeySizeTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasInsufficientKeySize" and
-    exists(DataFlow::Node source, DataFlow::Node sink |
-      exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlow(source, sink)) or
-      exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlow(source, sink)) or
-      exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlow(source, sink))
+    exists(DataFlow::PathNode source, DataFlow::PathNode sink |
+      exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
+      or
+      // exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or
+      // exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or
+      exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
     |
-      sink.getLocation() = location and
-      element = sink.toString() and
+      sink.getNode().getLocation() = location and
+      element = sink.getNode().toString() and
       value = ""
     )
   }