From ee7691f82e7deaf450bb029227e481cfbbdbfb7d Mon Sep 17 00:00:00 2001
From: aeneasr <3372410+aeneasr@users.noreply.github.com>
Date: Mon, 17 Aug 2020 20:30:17 +0200
Subject: [PATCH 1/2] Do not ignore decoding errors and other special cases

---
 token.go      | 12 +++++++++---
 token_test.go | 13 +++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/token.go b/token.go
index 3783dec..d9f7760 100644
--- a/token.go
+++ b/token.go
@@ -58,15 +58,21 @@ func b64decode(data string) []byte {
 // Supports masked tokens. realToken comes from Token(r) and
 // sentToken is token sent unusual way.
 func VerifyToken(realToken, sentToken string) bool {
-	r := b64decode(realToken)
+	r, err := base64.StdEncoding.DecodeString(realToken)
+	if err != nil {
+		return false
+	}
 	if len(r) == 2*tokenLength {
 		r = unmaskToken(r)
 	}
-	s := b64decode(sentToken)
+	s, err := base64.StdEncoding.DecodeString(sentToken)
+	if err != nil {
+		return false
+	}
 	if len(s) == 2*tokenLength {
 		s = unmaskToken(s)
 	}
-	return subtle.ConstantTimeCompare(r, s) == 1
+	return subtle.ConstantTimeCompare(r, s) == 1 && len(r) > 0 && len(s) > 0
 }
 
 func verifyToken(realToken, sentToken []byte) bool {
diff --git a/token_test.go b/token_test.go
index 2a848db..b669efd 100644
--- a/token_test.go
+++ b/token_test.go
@@ -70,3 +70,16 @@ func TestVerifiesMaskedTokenCorrectly(t *testing.T) {
 		t.Errorf("VerifyToken returned a false positive")
 	}
 }
+
+func TestVerifyTokenBase64Invalid(t *testing.T) {
+	for _, pairs := range [][]string{
+		{"foo", "bar"},
+		{"foo", ""},
+		{"", "bar"},
+		{"", ""},
+	} {
+		if VerifyToken(pairs[0], pairs[1]) {
+			t.Errorf("VerifyToken returned a false positive for: %v", pairs)
+		}
+	}
+}

From 0bc5e56a715eabf128e0a23f441e4fc447438ce8 Mon Sep 17 00:00:00 2001
From: Patrik <zepatrik@users.noreply.github.com>
Date: Thu, 27 Aug 2020 13:27:07 +0200
Subject: [PATCH 2/2] Use single source of truth for equality checks (#1)

Enforced a single point of token equality check for all functions. Added a test case to demonstrate why the verifyToken can not just be used.
---
 token.go      | 14 ++++++++------
 token_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/token.go b/token.go
index d9f7760..28d0172 100644
--- a/token.go
+++ b/token.go
@@ -72,9 +72,10 @@ func VerifyToken(realToken, sentToken string) bool {
 	if len(s) == 2*tokenLength {
 		s = unmaskToken(s)
 	}
-	return subtle.ConstantTimeCompare(r, s) == 1 && len(r) > 0 && len(s) > 0
+	return tokensEqual(r, s)
 }
 
+// verifyToken expects the realToken to be unmasked and the sentToken to be masked
 func verifyToken(realToken, sentToken []byte) bool {
 	realN := len(realToken)
 	sentN := len(sentToken)
@@ -83,15 +84,16 @@ func verifyToken(realToken, sentToken []byte) bool {
 	// sentN == 2*tokenLength means the token is masked.
 
 	if realN == tokenLength && sentN == 2*tokenLength {
-		return verifyMasked(realToken, sentToken)
+		return tokensEqual(realToken, unmaskToken(sentToken))
 	}
 	return false
 }
 
-// Verifies the masked token
-func verifyMasked(realToken, sentToken []byte) bool {
-	sentPlain := unmaskToken(sentToken)
-	return subtle.ConstantTimeCompare(realToken, sentPlain) == 1
+// tokensEqual expects both tokens to be unmasked
+func tokensEqual(realToken, sentToken []byte) bool {
+	return len(realToken) == tokenLength &&
+		len(sentToken) == tokenLength &&
+		subtle.ConstantTimeCompare(realToken, sentToken) == 1
 }
 
 func checkForPRNG() {
diff --git a/token_test.go b/token_test.go
index b669efd..66ddc18 100644
--- a/token_test.go
+++ b/token_test.go
@@ -2,6 +2,7 @@ package nosurf
 
 import (
 	"crypto/rand"
+	"encoding/base64"
 	"testing"
 )
 
@@ -83,3 +84,47 @@ func TestVerifyTokenBase64Invalid(t *testing.T) {
 		}
 	}
 }
+
+func TestVerifyTokenUnMasked(t *testing.T) {
+	for i, tc := range []struct {
+		real  string
+		send  string
+		valid bool
+	}{
+		{
+			real:  "qwertyuiopasdfghjklzxcvbnm123456",
+			send:  "qwertyuiopasdfghjklzxcvbnm123456",
+			valid: true,
+		},
+		{
+			real: "qwertyuiopasdfghjklzxcvbnm123456",
+			send: "qwertyuiopasdfghjklzxcvbnm123456" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+			valid: true,
+		},
+		{
+			real: "qwertyuiopasdfghjklzxcvbnm123456" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+			send: "qwertyuiopasdfghjklzxcvbnm123456" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+			valid: true,
+		},
+		{
+			real: "qwertyuiopasdfghjklzxcvbnm123456" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" +
+				"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+			send:  "qwertyuiopasdfghjklzxcvbnm123456",
+			valid: true,
+		},
+	} {
+		if VerifyToken(
+			base64.StdEncoding.EncodeToString([]byte(tc.real)),
+			base64.StdEncoding.EncodeToString([]byte(tc.send)),
+		) != tc.valid {
+			t.Errorf("Verify token returned wrong result for case %d: %+v", i, tc)
+		}
+	}
+}