Merge pull request #37723 from sberyozkin/csrf_double_sign_bug

Do not use CSRF cookie as the next token value
quarkusio · Dec 13, 2023 · c069f5f · c069f5f
2 parents 07c0c06 + 0e4097c
commit c069f5f
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 8 deletions.
diff --git a/...ime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java b/...ime/src/main/java/io/quarkus/csrf/reactive/runtime/CsrfRequestResponseReactiveFilter.java
@@ -66,8 +66,6 @@ public void filter(ResteasyReactiveContainerRequestContext requestContext, Routi
 
         String cookieToken = getCookieToken(routing, config);
         if (cookieToken != null) {
-            routing.put(CSRF_TOKEN_KEY, cookieToken);
-
             try {
                 int cookieTokenSize = Base64.getUrlDecoder().decode(cookieToken).length;
                 // HMAC SHA256 output is 32 bytes long
@@ -98,10 +96,10 @@ public void filter(ResteasyReactiveContainerRequestContext requestContext, Routi
             // unsafe HTTP method, token is required
 
             // Check the header first
-            String csrfTokenInHeader = requestContext.getHeaderString(config.tokenHeaderName);
-            if (csrfTokenInHeader != null) {
+            String csrfTokenHeaderParam = requestContext.getHeaderString(config.tokenHeaderName);
+            if (csrfTokenHeaderParam != null) {
                 LOG.debugf("CSRF token found in the token header");
-                verifyCsrfToken(requestContext, routing, config, cookieToken, csrfTokenInHeader);
+                verifyCsrfToken(requestContext, routing, config, cookieToken, csrfTokenHeaderParam);
                 return;
             }
 
@@ -128,9 +126,9 @@ public void filter(ResteasyReactiveContainerRequestContext requestContext, Routi
 
             ResteasyReactiveRequestContext rrContext = (ResteasyReactiveRequestContext) requestContext
                     .getServerRequestContext();
-            String csrfToken = (String) rrContext.getFormParameter(config.formFieldName, true, false);
+            String csrfTokenFormParam = (String) rrContext.getFormParameter(config.formFieldName, true, false);
             LOG.debugf("CSRF token found in the form parameter");
-            verifyCsrfToken(requestContext, routing, config, cookieToken, csrfToken);
+            verifyCsrfToken(requestContext, routing, config, cookieToken, csrfTokenFormParam);
             return;
 
         } else if (cookieToken == null) {
@@ -159,6 +157,7 @@ private void verifyCsrfToken(ResteasyReactiveContainerRequestContext requestCont
                 requestContext.abortWith(badClientRequest());
                 return;
             } else {
+                routing.put(CSRF_TOKEN_KEY, csrfToken);
                 routing.put(CSRF_TOKEN_VERIFIED, true);
                 return;
             }

diff --git a/integration-tests/csrf-reactive/src/main/java/io/quarkus/it/csrf/TestResource.java b/integration-tests/csrf-reactive/src/main/java/io/quarkus/it/csrf/TestResource.java
@@ -29,6 +29,12 @@ public class TestResource {
     @Inject
     Template csrfTokenForm;
 
+    @Inject
+    Template csrfTokenFirstForm;
+
+    @Inject
+    Template csrfTokenSecondForm;
+
     @Inject
     Template csrfTokenHeader;
 
@@ -49,6 +55,14 @@ public TemplateInstance getCsrfTokenForm() {
         return csrfTokenForm.instance();
     }
 
+    @GET
+    @Path("/csrfTokenFirstForm")
+    @Produces(MediaType.TEXT_HTML)
+    @Authenticated
+    public TemplateInstance getCsrfTokenFirstForm() {
+        return csrfTokenFirstForm.instance();
+    }
+
     @GET
     @Path("/csrfTokenWithFormRead")
     @Produces(MediaType.TEXT_HTML)
@@ -71,6 +85,22 @@ public String postCsrfTokenForm(@FormParam("name") String name, @HeaderParam("X-
         return name + ":" + routingContext.get("csrf_token_verified", false) + ":tokenHeaderIsSet=" + (csrfHeader != null);
     }
 
+    @POST
+    @Path("/csrfTokenFirstForm")
+    @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
+    @Produces(MediaType.TEXT_HTML)
+    public TemplateInstance postCsrfTokenFirstForm() {
+        return csrfTokenSecondForm.instance();
+    }
+
+    @POST
+    @Path("/csrfTokenSecondForm")
+    @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
+    @Produces(MediaType.TEXT_PLAIN)
+    public String postCsrfTokenSecondForm(@FormParam("name") String name, @HeaderParam("X-CSRF-TOKEN") String csrfHeader) {
+        return name + ":" + routingContext.get("csrf_token_verified", false) + ":tokenHeaderIsSet=" + (csrfHeader != null);
+    }
+
     @POST
     @Path("/csrfTokenWithFormRead")
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)

diff --git a/integration-tests/csrf-reactive/src/main/resources/application.properties b/integration-tests/csrf-reactive/src/main/resources/application.properties
@@ -1,5 +1,5 @@
 quarkus.csrf-reactive.cookie-name=csrftoken
-quarkus.csrf-reactive.create-token-path=/service/csrfTokenForm,/service/csrfTokenWithFormRead,/service/csrfTokenMultipart,/service/csrfTokenWithHeader
+quarkus.csrf-reactive.create-token-path=/service/csrfTokenForm,/service/csrfTokenFirstForm,/service/csrfTokenSecondForm,/service/csrfTokenWithFormRead,/service/csrfTokenMultipart,/service/csrfTokenWithHeader
 quarkus.csrf-reactive.token-signature-key=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
 
 quarkus.http.auth.basic=true

diff --git a/integration-tests/csrf-reactive/src/main/resources/templates/csrfTokenFirstForm.html b/integration-tests/csrf-reactive/src/main/resources/templates/csrfTokenFirstForm.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<title>CSRF Token First Form Test</title> 
+</head>
+<body>
+    <h1>CSRF Test</h1>
+
+    <form action="/service/csrfTokenFirstForm" method="post">
+    	<input type="hidden" name="{inject:csrf.parameterName}" value="{inject:csrf.token}" />
+
+    	<p>Your Name: <input type="text" name="name" /></p>
+    	<p><input type="submit" name="submit"/></p>
+    </form>
+</body>
+</html>
diff --git a/integration-tests/csrf-reactive/src/main/resources/templates/csrfTokenSecondForm.html b/integration-tests/csrf-reactive/src/main/resources/templates/csrfTokenSecondForm.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<title>CSRF Token Second Form Test</title> 
+</head>
+<body>
+    <h1>CSRF Test</h1>
+
+    <form action="/service/csrfTokenSecondForm" method="post">
+    	<input type="hidden" name="{inject:csrf.parameterName}" value="{inject:csrf.token}" />
+
+    	<p>Your Name: <input type="text" name="name" /></p>
+    	<p><input type="submit" name="submit"/></p>
+    </form>
+</body>
+</html>
diff --git a/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java b/integration-tests/csrf-reactive/src/test/java/io/quarkus/it/csrf/CsrfReactiveTest.java
@@ -61,6 +61,36 @@ public void testCsrfTokenInForm() throws Exception {
         }
     }
 
+    @Test
+    public void testCsrfTokenTwoForms() throws Exception {
+        try (final WebClient webClient = createWebClient()) {
+            webClient.addRequestHeader("Authorization", basicAuth("alice", "alice"));
+            HtmlPage htmlPage = webClient.getPage("http://localhost:8081/service/csrfTokenFirstForm");
+
+            assertEquals("CSRF Token First Form Test", htmlPage.getTitleText());
+
+            HtmlForm loginForm = htmlPage.getForms().get(0);
+
+            loginForm.getInputByName("name").setValueAttribute("alice");
+
+            assertNotNull(webClient.getCookieManager().getCookie("csrftoken"));
+
+            htmlPage = loginForm.getInputByName("submit").click();
+
+            assertEquals("CSRF Token Second Form Test", htmlPage.getTitleText());
+
+            loginForm = htmlPage.getForms().get(0);
+
+            loginForm.getInputByName("name").setValueAttribute("alice");
+
+            TextPage textPage = loginForm.getInputByName("submit").click();
+            assertNotNull(webClient.getCookieManager().getCookie("csrftoken"));
+            assertEquals("alice:true:tokenHeaderIsSet=false", textPage.getContent());
+
+            webClient.getCookieManager().clearCookies();
+        }
+    }
+
     @Test
     public void testCsrfTokenWithFormRead() throws Exception {
         try (final WebClient webClient = createWebClient()) {