Use a full dataflow config rather than local flow

Author: joefarebrother

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

@@ -4,12 +4,20 @@ import java
 import Encryption
 import semmle.code.java.dataflow.DataFlow
 
-/** Holds if `c` is a call which initialises an RSA cipher without using OAEP padding. */
-predicate rsaWithoutOaepCall(CryptoAlgoSpec c) {
-  exists(CompileTimeConstantExpr specExpr, string spec |
-    specExpr.getStringValue() = spec and
-    DataFlow::localExprFlow(specExpr, c.getAlgoSpec()) and
-    spec.matches("RSA/%") and
-    not spec.matches("%OAEP%")
-  )
+/** A configuration for finding RSA ciphers initialized without using OAEP padding. */
+class RsaWithoutOaepConfig extends DataFlow::Configuration {
+  RsaWithoutOaepConfig() { this = "RsaWithoutOaepConfig" }
+
+  override predicate isSource(DataFlow::Node src) {
+    exists(CompileTimeConstantExpr specExpr, string spec |
+      specExpr.getStringValue() = spec and
+      specExpr = src.asExpr() and
+      spec.matches("RSA/%") and
+      not spec.matches("%OAEP%")
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(CryptoAlgoSpec cr | sink.asExpr() = cr.getAlgoSpec())
+  }
 }

java/ql/src/Security/CWE/CWE-780/RsaWithoutOaep.ql

@@ -1,7 +1,7 @@
 /**
  * @name Use of RSA algorithm without OAEP
  * @description Using RSA encryption without OAEP padding can lead to a padding oracle attack, weakening the encryption.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @security-severity 7.5
  * @precision high
@@ -11,9 +11,10 @@
  */
 
 import java
-import semmle.code.java.security.Encryption
 import semmle.code.java.security.RsaWithoutOaepQuery
+import DataFlow::PathGraph
 
-from CryptoAlgoSpec c
-where rsaWithoutOaepCall(c)
-select c, "This instance of RSA does not use OAEP padding."
+from RsaWithoutOaepConfig conf, DataFlow::Node source, DataFlow::Node sink
+where conf.hasFlow(source, sink)
+select source, source, sink,
+  "This specification is used to initialize an RSA cipher without OAEP padding $@.", sink, "here"

java/ql/test/query-tests/security/CWE-780/RsaWithoutOaepTest.java

@@ -2,8 +2,16 @@
 
 class RsaWithoutOaep {
     public void test() throws Exception {
-        Cipher rsaBad = Cipher.getInstance("RSA/ECB/NoPadding"); // $hasResult
+        Cipher rsaBad = Cipher.getInstance("RSA/ECB/NoPadding"); // $hasTaintFlow
 
         Cipher rsaGood = Cipher.getInstance("RSA/ECB/OAEPWithSHA-1AndMGF1Padding"); 
     }
+
+    public Cipher getCipher(String spec) throws Exception {
+        return Cipher.getInstance(spec); // $hasTaintFlow
+    }
+
+    public void test2() throws Exception {
+        Cipher rsa = getCipher("RSA/ECB/NoPadding");
+    }
 }

java/ql/test/query-tests/security/CWE-780/RsaWithoutOaepTest.ql

@@ -1,19 +1,10 @@
 import java
 import TestUtilities.InlineExpectationsTest
+import TestUtilities.InlineFlowTest
 import semmle.code.java.security.RsaWithoutOaepQuery
 
-class HasResult extends InlineExpectationsTest {
-  HasResult() { this = "HasResult" }
+class HasFlowTest extends InlineFlowTest {
+  override DataFlow::Configuration getTaintFlowConfig() { result instanceof RsaWithoutOaepConfig }
 
-  override string getARelevantTag() { result = "hasResult" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
-    tag = "hasResult" and
-    value = "" and
-    exists(CryptoAlgoSpec c |
-      rsaWithoutOaepCall(c) and
-      location = c.getLocation() and
-      element = c.toString()
-    )
-  }
+  override DataFlow::Configuration getValueFlowConfig() { none() }
 }