From 534ab86900bde6df0ce1bf1418f7286d07ea8bcf Mon Sep 17 00:00:00 2001
From: Sauyon Lee <sauyon@github.com>
Date: Fri, 2 Apr 2021 01:13:41 -0700
Subject: [PATCH] Add models for Spring validation.Errors

---
 ...2021-04-02-add-spring-validation-errors.md |  2 +
 .../code/java/frameworks/spring/Spring.qll    |  1 +
 .../java/frameworks/spring/SpringErrors.qll   | 22 ++++++
 .../spring/ValidationErrorsTest.java          | 72 +++++++++++++++++++
 4 files changed, 97 insertions(+)
 create mode 100644 java/change-notes/2021-04-02-add-spring-validation-errors.md
 create mode 100644 java/ql/src/semmle/code/java/frameworks/spring/SpringErrors.qll
 create mode 100644 java/ql/test/library-tests/frameworks/spring/ValidationErrorsTest.java

diff --git a/java/change-notes/2021-04-02-add-spring-validation-errors.md b/java/change-notes/2021-04-02-add-spring-validation-errors.md
new file mode 100644
index 00000000000..d3f7ee6e958
--- /dev/null
+++ b/java/change-notes/2021-04-02-add-spring-validation-errors.md
@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added additional taint steps modeling the Spring `validation.Errors` class (`org.springframework.validation.Errors`).
diff --git a/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll b/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll
index 2b4ad64b1f6..aca9fb1188d 100644
--- a/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll
+++ b/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll
@@ -12,6 +12,7 @@ import semmle.code.java.frameworks.spring.SpringConstructorArg
 import semmle.code.java.frameworks.spring.SpringController
 import semmle.code.java.frameworks.spring.SpringDescription
 import semmle.code.java.frameworks.spring.SpringEntry
+import semmle.code.java.frameworks.spring.SpringErrors
 import semmle.code.java.frameworks.spring.SpringFlex
 import semmle.code.java.frameworks.spring.SpringIdRef
 import semmle.code.java.frameworks.spring.SpringImport
diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringErrors.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringErrors.qll
new file mode 100644
index 00000000000..48594d077b6
--- /dev/null
+++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringErrors.qll
@@ -0,0 +1,22 @@
+/** Definitions of flow steps through utility methods of `org.springframework.validation.Errors`. */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+
+private class SpringErrorModel extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.springframework.validation;Errors;true;addAllErrors;;;Argument[0];Argument[-1];taint",
+        "org.springframework.validation;Errors;true;getAllErrors;;;Argument[-1];ReturnValue;taint",
+        "org.springframework.validation;Errors;true;getFieldError;;;Argument[-1];ReturnValue;taint",
+        "org.springframework.validation;Errors;true;getFieldErrors;;;Argument[-1];ReturnValue;taint",
+        "org.springframework.validation;Errors;true;getGlobalError;;;Argument[-1];ReturnValue;taint",
+        "org.springframework.validation;Errors;true;getGlobalErrors;;;Argument[-1];ReturnValue;taint",
+        "org.springframework.validation;Errors;true;reject;;;Argument[0..2];Argument[-1];taint",
+        "org.springframework.validation;Errors;true;rejectValue;;;Argument[1..3];Argument[-1];taint",
+        "org.springframework.validation;Errors;true;reject;;;Argument[0..2];Argument[-1];taint",
+        "org.springframework.validation;Errors;true;rejectValue;;;Argument[1..3];Argument[-1];taint"
+      ]
+  }
+}
diff --git a/java/ql/test/library-tests/frameworks/spring/ValidationErrorsTest.java b/java/ql/test/library-tests/frameworks/spring/ValidationErrorsTest.java
new file mode 100644
index 00000000000..1106f347f73
--- /dev/null
+++ b/java/ql/test/library-tests/frameworks/spring/ValidationErrorsTest.java
@@ -0,0 +1,72 @@
+import org.springframework.validation.Errors;
+
+class ValidationErrorsTest {
+  String taint() { return "tainted"; }
+
+  Errors taintErrs() { return null; }
+  Errors errors() { return null; }
+
+  void sink(Object o) {}
+
+  void test() {
+    Errors es0 = errors();
+    es0.addAllErrors(taintErrs());
+    sink(es0); // $hasTaintFlow
+
+    sink(taintErrs().getAllErrors()); // $hasTaintFlow
+
+    sink(taintErrs().getFieldError()); // $hasTaintFlow
+    sink(taintErrs().getFieldError("field")); // $hasTaintFlow
+
+    sink(taintErrs().getGlobalError()); // $hasTaintFlow
+    sink(taintErrs().getGlobalErrors()); // $hasTaintFlow
+
+    Errors es1 = errors();
+    es1.reject(taint());
+    sink(es1); // $hasTaintFlow
+
+    Errors es2 = errors();
+    es2.reject(taint(), null, "");
+    sink(es2); // $hasTaintFlow
+
+    Errors es3 = errors();
+    es3.reject(taint(), new Object[]{}, "");
+    sink(es3); // $hasTaintFlow
+
+    Errors es4 = errors();
+    es4.reject("", new Object[]{taint()}, "");
+    sink(es4); // $hasTaintFlow
+
+    Errors es5 = errors();
+    es5.reject("", new Object[]{}, taint());
+    sink(es5); // $hasTaintFlow
+
+    Errors es6 = errors();
+    es6.reject(taint(), "");
+    sink(es6); // $hasTaintFlow
+
+    Errors es7 = errors();
+    es7.reject("", taint());
+    sink(es7); // $hasTaintFlow
+
+    Errors es8 = errors();
+    es8.rejectValue("", taint(), new Object[]{}, "");
+    sink(es8); // $hasTaintFlow
+
+    Errors es9 = errors();
+    es9.rejectValue("", "", new Object[]{taint()}, "");
+    sink(es9); // $hasTaintFlow
+
+    Errors es10 = errors();
+    es10.rejectValue("", "", new Object[]{}, taint());
+    sink(es10); // $hasTaintFlow
+
+    Errors es11 = errors();
+    es11.rejectValue("", taint(), "");
+    sink(es11); // $hasTaintFlow
+
+    Errors es12 = errors();
+    es12.rejectValue("", "", taint());
+    sink(es12); // $hasTaintFlow
+  }
+}