fix: use time-constant comparison for security tokens (#12189)

This is the same as https://github.com/vaadin/framework/pull/12188,
but also applied for the upload security key
and the push id since both of those are also used to protect against
cross-site attacks. In addition, documentation for the push id is
clarified to point out its role.

Cherry-picked from: https://github.com/vaadin/flow/pull/9896

diff --git a/server/src/main/java/com/vaadin/server/VaadinSession.java b/server/src/main/java/com/vaadin/server/VaadinSession.java
index dd64de4fa59bdd9a972704398277d4abb45c3464..6f25fdc56f75ad18606b2cf04c684979f8c0fba8 100644 (file)
--- a/server/src/main/java/com/vaadin/server/VaadinSession.java
+++ b/server/src/main/java/com/vaadin/server/VaadinSession.java
@@ -743,6 +743,10 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
      */
     private final String csrfToken = UUID.randomUUID().toString();
 
+    /*
+     * This token should be handled with care since it's used to protect against
+     * cross-site attacks in addition to general identifier duty.
+     */
     private final String pushId = UUID.randomUUID().toString();
 
     /**

diff --git a/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java b/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
index bd20bea9ed618fc87969c2795c91674ef9493aa9..81cd21c813dc492569e09de28c46298e7868e20e 100644 (file)
--- a/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
+++ b/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
@@ -23,6 +23,8 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 
 import com.vaadin.server.ClientConnector;
 import com.vaadin.server.NoInputStreamException;
@@ -279,7 +281,10 @@ public class FileUploadHandler implements RequestHandler {
             streamVariable = uI.getConnectorTracker()
                     .getStreamVariable(connectorId, variableName);
             String secKey = uI.getConnectorTracker().getSeckey(streamVariable);
-            if (secKey == null || !secKey.equals(parts[3])) {
+            String securityKey = parts[3];
+            if (secKey == null || !MessageDigest.isEqual(
+                    secKey.getBytes(StandardCharsets.UTF_8),
+                    securityKey.getBytes(StandardCharsets.UTF_8))) {
                 return true;
             }
 

diff --git a/server/src/main/java/com/vaadin/server/communication/PushHandler.java b/server/src/main/java/com/vaadin/server/communication/PushHandler.java
index 77dd1b9b57ef3a04084ade357d2275a5dd4dbad2..f1997ff7cf5f891ba9fca837ccc400718b1e2037 100644 (file)
--- a/server/src/main/java/com/vaadin/server/communication/PushHandler.java
+++ b/server/src/main/java/com/vaadin/server/communication/PushHandler.java
@@ -18,6 +18,8 @@ package com.vaadin.server.communication;
 
 import java.io.IOException;
 import java.io.Reader;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 import java.util.Collection;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -510,7 +512,9 @@ public class PushHandler {
     }
 
     /**
-     * Checks whether a given push id matches the session's push id.
+     * Checks whether a given push id matches the session's push id. The
+     * comparison is done using a time-constant method since the push id is used
+     * to protect against cross-site attacks.
      *
      * @param session
      *            the vaadin session for which the check should be done
@@ -522,7 +526,9 @@ public class PushHandler {
             String requestPushId) {
 
         String sessionPushId = session.getPushId();
-        if (requestPushId == null || !requestPushId.equals(sessionPushId)) {
+        if (requestPushId == null || !MessageDigest.isEqual(
+                requestPushId.getBytes(StandardCharsets.UTF_8),
+                sessionPushId.getBytes(StandardCharsets.UTF_8))) {
             return false;
         }
         return true;