Merge pull request #14491 from egregius313/egregius313/java/mad/convert-iv

Java: Refactor `java/static-initialization-vector` to use Models as Data

Author: egregius313

java/ql/lib/ext/javax.crypto.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["javax.crypto", "Cipher", True, "init", "(int,Key,AlgorithmParameterSpec)", "", "Argument[2]", "encryption-iv", "manual"]
+      - ["javax.crypto", "Cipher", True, "init", "(int,Key,AlgorithmParameterSpec,SecureRandom)", "", "Argument[2]", "encryption-iv", "manual"]

java/ql/lib/ext/javax.crypto.spec.model.yml

@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: summaryModel
+    data:
+      - ["javax.crypto.spec", "IvParameterSpec", True, "IvParameterSpec", "", "", "Argument[0]", "Argument[this]", "taint", "manual"]
+      - ["javax.crypto.spec", "GCMParameterSpec", True, "GCMParameterSpec", "", "", "Argument[1]", "Argument[this]", "taint", "manual"]
+      - ["javax.crypto.spec", "RC2ParameterSpec", True, "RC2ParameterSpec", "", "", "Argument[1]", "Argument[this]", "taint", "manual"]
+      - ["javax.crypto.spec", "RC5ParameterSpec", True, "RC5ParameterSpec", "", "", "Argument[3]", "Argument[this]", "taint", "manual"]

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

@@ -2,7 +2,7 @@
 
 import java
 import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.DataFlow2
+private import semmle.code.java.dataflow.ExternalFlow
 
 /**
  * Holds if `array` is initialized only with constants.
@@ -99,7 +99,7 @@ private module ArrayUpdateFlow = DataFlow::Global<ArrayUpdateConfig>;
 private class StaticInitializationVectorSource extends DataFlow::Node {
   StaticInitializationVectorSource() {
     exists(StaticByteArrayCreation array | array = this.asExpr() |
-      not ArrayUpdateFlow::flow(DataFlow2::exprNode(array), _) and
+      not ArrayUpdateFlow::flow(DataFlow::exprNode(array), _) and
       // Reduce FPs from utility methods that return an empty array in an exceptional case
       not exists(ReturnStmt ret |
         array.getADimension().(CompileTimeConstantExpr).getIntValue() = 0 and
@@ -113,34 +113,7 @@ private class StaticInitializationVectorSource extends DataFlow::Node {
  * A sink that initializes a cipher with unsafe parameters.
  */
 private class EncryptionInitializationSink extends DataFlow::Node {
-  EncryptionInitializationSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m.hasQualifiedName("javax.crypto", "Cipher", "init") and
-      m.getParameterType(2)
-          .(RefType)
-          .hasQualifiedName("java.security.spec", "AlgorithmParameterSpec") and
-      ma.getArgument(2) = this.asExpr()
-    )
-  }
-}
-
-/**
- * Holds if `fromNode` to `toNode` is a dataflow step
- * that creates cipher's parameters with initialization vector.
- */
-private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-  exists(ConstructorCall cc, RefType type |
-    cc = toNode.asExpr() and type = cc.getConstructedType()
-  |
-    type.hasQualifiedName("javax.crypto.spec", "IvParameterSpec") and
-    cc.getArgument(0) = fromNode.asExpr()
-    or
-    type.hasQualifiedName("javax.crypto.spec", ["GCMParameterSpec", "RC2ParameterSpec"]) and
-    cc.getArgument(1) = fromNode.asExpr()
-    or
-    type.hasQualifiedName("javax.crypto.spec", "RC5ParameterSpec") and
-    cc.getArgument(3) = fromNode.asExpr()
-  )
+  EncryptionInitializationSink() { sinkNode(this, "encryption-iv") }
 }
 
 /**
@@ -156,10 +129,6 @@ deprecated class StaticInitializationVectorConfig extends TaintTracking::Configu
   }
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-    createInitializationVectorSpecStep(fromNode, toNode)
-  }
 }
 
 /**
@@ -169,10 +138,6 @@ module StaticInitializationVectorConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof StaticInitializationVectorSource }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink }
-
-  predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-    createInitializationVectorSpecStep(fromNode, toNode)
-  }
 }
 
 /** Tracks the flow from a static initialization vector to the initialization of a cipher */