Skip to content

Commit

Permalink
fix: use time-constant comparison for security tokens (#9896)
Browse files Browse the repository at this point in the history
This is the same as #9875, 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 commit 088293f)
  • Loading branch information
Legioth authored and pleku committed Jan 28, 2021
1 parent 67c73e1 commit 00fe432
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package com.vaadin.flow.server;

import javax.servlet.http.HttpSession;
import javax.servlet.http.HttpSessionBindingEvent;
import javax.servlet.http.HttpSessionBindingListener;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.Serializable;
Expand All @@ -36,10 +40,6 @@
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import javax.servlet.http.HttpSession;
import javax.servlet.http.HttpSessionBindingEvent;
import javax.servlet.http.HttpSessionBindingListener;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -119,6 +119,18 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
*/
private transient ConcurrentLinkedQueue<FutureAccess> pendingAccessQueue = new ConcurrentLinkedQueue<>();

/*
* Despite section 6 of RFC 4122, this particular use of UUID *is* adequate
* for security capabilities. Type 4 UUIDs contain 122 bits of random data,
* and UUID.randomUUID() is defined to use a cryptographically secure random
* generator.
*/
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();

private final Attributes attributes = new Attributes();
Expand Down Expand Up @@ -1030,4 +1042,14 @@ public StreamResourceRegistry getResourceRegistry() {
return resourceRegistry;
}

/**
* Gets the CSRF token (aka double submit cookie) that is used to protect
* against Cross Site Request Forgery attacks.
*
* @return the csrf token string
* @since 2.0
*/
public String getCsrfToken() {
return csrfToken;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import java.io.IOException;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.Collection;

import org.atmosphere.cpr.AtmosphereRequest;
Expand Down Expand Up @@ -480,7 +482,9 @@ private static Logger getLogger() {
}

/**
* 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
Expand All @@ -492,7 +496,9 @@ private static boolean isPushIdValid(VaadinSession session,
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Serializable;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.Collection;
import java.util.Iterator;

Expand Down Expand Up @@ -119,7 +121,9 @@ public void handleRequest(VaadinSession session, VaadinRequest request,
session.lock();
try {
String secKey = streamReceiver.getId();
if (secKey == null || !secKey.equals(securityKey)) {
if (secKey == null || !MessageDigest.isEqual(
secKey.getBytes(StandardCharsets.UTF_8),
securityKey.getBytes(StandardCharsets.UTF_8))) {
getLogger().warn(
"Received incoming stream with faulty security key.");
return;
Expand Down

0 comments on commit 00fe432

Please sign in to comment.