Skip to content

Commit

Permalink
[bugfix] Token comparison could fail on versions of Go < 1.3.
Browse files Browse the repository at this point in the history
- subtle.ConstantTimeCompare did not check for matching slice lengths prior to Go
  1.3 (fixed in https://codereview.appspot.com/118750043).
- goji/csrf was released a year after this came into place.
- Our TravisCI tests did not test against older versions of Go, and this wasn't
  caught as a result.
- Have added Go 1.2 and Go 1.3 to the TravisCI config to address any future
  issues.
  • Loading branch information
elithrar committed Feb 13, 2016
1 parent 92a804c commit 22f03b5
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
8 changes: 5 additions & 3 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,13 @@ func sameOrigin(a, b *url.URL) bool {
// compare securely (constant-time) compares the unmasked token from the request
// against the real token from the session.
func compareTokens(a, b []byte) bool {
if subtle.ConstantTimeCompare(a, b) == 1 {
return true
// This is required as subtle.ConstantTimeCompare does not check for equal
// lengths in Go versions prior to 1.3.
if len(a) != len(b) {
return false
}

return false
return subtle.ConstantTimeCompare(a, b) == 1
}

// xorToken XORs tokens ([]byte) to provide unique-per-request CSRF tokens. It
Expand Down
11 changes: 11 additions & 0 deletions helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,14 @@ func TestTemplateField(t *testing.T) {
customTemplateField, expectedTemplateField)
}
}

func TestCompareTokens(t *testing.T) {
// Go's subtle.ConstantTimeCompare prior to 1.3 did not check for matching
// lengths.
a := []byte("")
b := []byte("an-actual-token")

if v := compareTokens(a, b); v == true {
t.Fatalf("compareTokens failed on different tokens: got %v want %v", v, !v)
}
}

0 comments on commit 22f03b5

Please sign in to comment.