Fix CSRF protection bypass #74

softwaremill · Mar 11, 2020 · 57f1166 · 57f1166
1 parent ed2567c
commit 57f1166
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 1 deletion.
diff --git a/core/src/main/scala/com/softwaremill/session/CsrfDirectives.scala b/core/src/main/scala/com/softwaremill/session/CsrfDirectives.scala
@@ -23,7 +23,7 @@ trait CsrfDirectives {
         // that the token matches.
         get.recover { _ =>
           submittedCsrfToken(checkMode).flatMap { submitted =>
-            if (submitted == cookie) {
+            if (submitted == cookie && !cookie.isEmpty) {
               pass
             } else {
               reject(checkMode.csrfManager.tokenInvalidRejection).toDirective[Unit]

diff --git a/core/src/test/scala/com/softwaremill/session/CsrfDirectivesTest.scala b/core/src/test/scala/com/softwaremill/session/CsrfDirectivesTest.scala
@@ -81,6 +81,20 @@ class CsrfDirectivesTest extends FlatSpec with ScalatestRouteTest with Matchers
     }
   }
 
+  it should "reject requests if the csrf cookie and the header are empty" in {
+    Get("/site") ~> routes ~> check {
+      responseAs[String] should be("ok")
+
+      Post("/transfer_money") ~>
+        addHeader(Cookie(cookieName, "")) ~>
+        addHeader(sessionConfig.csrfSubmittedName, "") ~>
+        routes ~>
+        check {
+          rejections should be(List(AuthorizationFailedRejection))
+        }
+    }
+  }
+
   it should "accept requests if the csrf cookie matches the header value" in {
     Get("/site") ~> routes ~> check {
       responseAs[String] should be("ok")

diff --git a/javaTests/src/test/java/com/softwaremill/session/javadsl/CsrfDirectivesTest.java b/javaTests/src/test/java/com/softwaremill/session/javadsl/CsrfDirectivesTest.java
@@ -135,6 +135,33 @@ public void shouldRejectRequestsIfTheCsrfCookieIsNotSet() {
 
     }
 
+    @Test
+    public void shouldRejectRequestsIfTheCsrfCookieAndTheHeaderAreEmpty() {
+        // given
+        final Route route = createCsrfRouteWithCheckHeaderMode();
+
+        // when
+        TestRouteResult testRouteResult = testRoute(route)
+          .run(HttpRequest.GET("/site"));
+
+        // then
+        testRouteResult
+          .assertStatusCode(StatusCodes.OK);
+
+        /* second request */
+        // when
+        TestRouteResult testRouteResult2 = testRoute(route)
+          .run(HttpRequest.POST("/transfer_money")
+            .addHeader(Cookie.create(csrfCookieName, ""))
+            .addHeader(RawHeader.create(csrfSubmittedName, ""))
+          );
+
+        // then
+        testRouteResult2
+          .assertStatusCode(StatusCodes.FORBIDDEN);
+
+    }
+
     @Test
     public void shouldAcceptRequestsIfTheCsrfCookieMatchesTheHeaderValue() {
         // given