From 5b1858a51474cd4e2368d40447b9d7517d87a0ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mun=CC=83oz?= <alvaro@pwntester.com>
Date: Wed, 11 Nov 2020 11:50:15 +0100
Subject: [PATCH] Do not report the issue only if all message interpolators are
 secure

---
 .../CWE/CWE-094/InsecureBeanValidation.ql       | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql b/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql
index efac203a724..d4552b44310 100644
--- a/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql
+++ b/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql
@@ -27,11 +27,10 @@ class ELMessageInterpolatorType extends RefType {
 }
 
 /**
- * A method call that sets the application's default message interpolator to an interpolator type that is likely to be safe,
- * because it does not process Java Expression Language expressions.
+ * A method call that sets the application's default message interpolator.
  */
-class SetSafeMessageInterpolatorCall extends MethodAccess {
-  SetSafeMessageInterpolatorCall() {
+class SetMessageInterpolatorCall extends MethodAccess {
+  SetMessageInterpolatorCall() {
     exists(Method m, RefType t |
       this.getMethod() = m and
       m.getDeclaringType().getASourceSupertype*() = t and
@@ -44,7 +43,13 @@ class SetSafeMessageInterpolatorCall extends MethodAccess {
               ["CustomValidatorBean", "LocalValidatorFactoryBean"]) and
         m.getName() = "setMessageInterpolator"
       )
-    ) and
+    )
+  }
+
+  /**
+  * The message interpolator is likely to be safe, because it does not process Java Expression Language expressions.
+  */
+  predicate isSafe() {
     not this.getAnArgument().getType() instanceof ELMessageInterpolatorType
   }
 }
@@ -82,7 +87,7 @@ class BeanValidationConfig extends TaintTracking::Configuration {
 
 from BeanValidationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where
-  not exists(SetSafeMessageInterpolatorCall ma) and
+  not forall(SetMessageInterpolatorCall c | c.isSafe()) and
   cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
   "Custom constraint error message contains unsanitized user data"