From ab49397bb80862e8572f1421eff612a9d1828793 Mon Sep 17 00:00:00 2001
From: Grzegorz Golawski <grzegol@gmail.com>
Date: Fri, 3 Jan 2020 21:52:50 +0100
Subject: [PATCH 1/4] Add check for disabled CSRF protection in Spring

---
 .../CWE/CWE-352/SpringCSRFProtection.java     | 17 ++++++++
 .../CWE/CWE-352/SpringCSRFProtection.qhelp    | 39 +++++++++++++++++++
 .../CWE/CWE-352/SpringCSRFProtection.ql       | 22 +++++++++++
 3 files changed, 78 insertions(+)
 create mode 100644 java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.java
 create mode 100644 java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
 create mode 100644 java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql

diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.java b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.java
new file mode 100644
index 000000000000..018c16ecadde
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.java
@@ -0,0 +1,17 @@
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
+import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+
+@EnableWebSecurity
+@Configuration
+public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
+  @Override
+  protected void configure(HttpSecurity http) throws Exception {
+    http
+      .csrf(csrf ->
+        // BAD - CSRF protection shouldn't be disabled
+        csrf.disable() 
+      );
+  }
+}
diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
new file mode 100644
index 000000000000..cae2ae4b7d2e
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
@@ -0,0 +1,39 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>When a web server is designed to receive a request from a client without any mechanism 
+for verifying that it was intentionally sent, then it might be possible for an attacker 
+to trick a client into making an unintentional request to the web server which will be treated 
+as an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can 
+result in exposure of data or unintended code execution.</p>
+</overview>
+
+<recommendation>
+<p>Cross-Site Request Forgery (CSRF) protection is enabled by default in Spring with Java 
+configuration. It's recommended to not disable this.</p>
+</recommendation>
+
+<example>
+<p>The following example shows the Spring Java configuration with CSRF protection disabled.</p>
+
+<sample src="SpringCSRFProtection.java" />
+</example>
+
+<references>
+<li>
+CWE:
+<a href="https://cwe.mitre.org/data/definitions/352.html">CWE-352: Cross-Site Request Forgery (CSRF)</a>.
+</li>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)">Cross-Site Request Forgery (CSRF)</a>.
+</li>
+<li>
+Spring Security Reference:
+<a href="https://docs.spring.io/spring-security/site/docs/current/reference/html5/#servlet-csrf">
+  Cross Site Request Forgery (CSRF) for Servlet Environments
+</a>.
+</li>
+</references>
+</qhelp>
\ No newline at end of file
diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
new file mode 100644
index 000000000000..f41532c8e073
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
@@ -0,0 +1,22 @@
+/**
+ * @name Disabled Spring CSRF protection
+ * @description Disabling CSRF protection makes the application vulnerable to
+ *              Cross-Site Request Forgery (CSRF) attack.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id java/spring-disabled-csrf-protection
+ * @tags security
+ *       external/cwe/cwe-352
+ */
+
+import java
+
+from MethodAccess call, Method method
+where
+  call.getMethod() = method and
+  method.hasName("disable") and
+  method.getDeclaringType().getQualifiedName().regexpMatch(
+    "org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer<CsrfConfigurer<.*>,.*>"
+  )
+select call, "CSRF vulnerability due to protection being disabled."

From 4ce25c045d77f1293a13e51e31d54e1da5503d60 Mon Sep 17 00:00:00 2001
From: Grzegorz Golawski <grzegol@gmail.com>
Date: Sun, 5 Jan 2020 22:05:00 +0100
Subject: [PATCH 2/4] Simplify the query

---
 .../src/Security/CWE/CWE-352/SpringCSRFProtection.ql   | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
index f41532c8e073..9529aa9aabf0 100644
--- a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
@@ -12,11 +12,11 @@
 
 import java
 
-from MethodAccess call, Method method
+from MethodAccess call
 where
-  call.getMethod() = method and
-  method.hasName("disable") and
-  method.getDeclaringType().getQualifiedName().regexpMatch(
-    "org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer<CsrfConfigurer<.*>,.*>"
+  call.getMethod().hasName("disable") and
+  call.getReceiverType().hasQualifiedName(
+    "org.springframework.security.config.annotation.web.configurers",
+    "CsrfConfigurer<HttpSecurity>"
   )
 select call, "CSRF vulnerability due to protection being disabled."

From c5a974788b68f46d420af79eee4dfff85542d329 Mon Sep 17 00:00:00 2001
From: Grzegorz Golawski <grzegol@gmail.com>
Date: Tue, 21 Jan 2020 21:54:36 +0100
Subject: [PATCH 3/4] Add check for disabled CSRF protection in Spring

Fix the help according to review comments.
---
 .../CWE/CWE-352/SpringCSRFProtection.qhelp    | 21 +++++++++----------
 .../CWE/CWE-352/SpringCSRFProtection.ql       |  2 +-
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
index cae2ae4b7d2e..98a7a1852a0f 100644
--- a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
@@ -2,30 +2,29 @@
 <qhelp>
 
 <overview>
-<p>When a web server is designed to receive a request from a client without any mechanism 
-for verifying that it was intentionally sent, then it might be possible for an attacker 
-to trick a client into making an unintentional request to the web server which will be treated 
-as an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can 
+<p>When you set up a web server to receive a request from a client without any mechanism
+for verifying that it was intentionally sent, then it is vulnerable to attack. An attacker can
+trick a client into making an unintended request to the web server that will be treated as
+an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can 
 result in exposure of data or unintended code execution.</p>
 </overview>
 
 <recommendation>
-<p>Cross-Site Request Forgery (CSRF) protection is enabled by default in Spring with Java 
-configuration. It's recommended to not disable this.</p>
+<p>Cross-Site Request Forgery (CSRF) protection is enabled by default. Spring's recommendation
+is to use CSRF protection for any request that could be processed by a browser client by normal
+users.</p>
 </recommendation>
 
 <example>
-<p>The following example shows the Spring Java configuration with CSRF protection disabled.</p>
+<p>The following example shows the Spring Java configuration with CSRF protection disabled.
+This type of configuration should only be used if you are creating a service that is used only
+by non-browser clients.</p>
 
 <sample src="SpringCSRFProtection.java" />
 </example>
 
 <references>
 <li>
-CWE:
-<a href="https://cwe.mitre.org/data/definitions/352.html">CWE-352: Cross-Site Request Forgery (CSRF)</a>.
-</li>
-<li>
 OWASP:
 <a href="https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)">Cross-Site Request Forgery (CSRF)</a>.
 </li>
diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
index 9529aa9aabf0..ecb536153a1c 100644
--- a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
@@ -1,7 +1,7 @@
 /**
  * @name Disabled Spring CSRF protection
  * @description Disabling CSRF protection makes the application vulnerable to
- *              Cross-Site Request Forgery (CSRF) attack.
+ *              a Cross-Site Request Forgery (CSRF) attack.
  * @kind problem
  * @problem.severity error
  * @precision high

From 5596944926d06f6ca28e78adc9a8f87e41e0e5d2 Mon Sep 17 00:00:00 2001
From: Grzegorz Golawski <grzegol@gmail.com>
Date: Wed, 22 Jan 2020 21:27:34 +0100
Subject: [PATCH 4/4] Add check for disabled CSRF protection in Spring

Fix help and correct formatting.
---
 .../src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp   | 2 +-
 java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql  | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
index 98a7a1852a0f..af1119bcd1c5 100644
--- a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.qhelp
@@ -10,7 +10,7 @@ result in exposure of data or unintended code execution.</p>
 </overview>
 
 <recommendation>
-<p>Cross-Site Request Forgery (CSRF) protection is enabled by default. Spring's recommendation
+<p>When you use Spring, Cross-Site Request Forgery (CSRF) protection is enabled by default. Spring's recommendation
 is to use CSRF protection for any request that could be processed by a browser client by normal
 users.</p>
 </recommendation>
diff --git a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
index ecb536153a1c..01438894b33d 100644
--- a/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
+++ b/java/ql/src/Security/CWE/CWE-352/SpringCSRFProtection.ql
@@ -15,8 +15,8 @@ import java
 from MethodAccess call
 where
   call.getMethod().hasName("disable") and
-  call.getReceiverType().hasQualifiedName(
-    "org.springframework.security.config.annotation.web.configurers",
-    "CsrfConfigurer<HttpSecurity>"
-  )
+  call
+      .getReceiverType()
+      .hasQualifiedName("org.springframework.security.config.annotation.web.configurers",
+        "CsrfConfigurer<HttpSecurity>")
 select call, "CSRF vulnerability due to protection being disabled."