Skip to content

Commit

Permalink
Merge pull request #8 from goji/compare-token-fix
Browse files Browse the repository at this point in the history
[bugfix] Compare token fix

- 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.
- Ref: https://docs.travis-ci.com/user/speeding-up-the-build/
  • Loading branch information
elithrar committed Feb 13, 2016
2 parents 92a804c + 8fe8706 commit 94b4359
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
16 changes: 11 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
language: go
sudo: false
go:
- 1.4
- 1.5
- tip

matrix:
include:
- go: 1.2
- go: 1.3
- go: 1.4
- go: 1.5
- go: tip

install:
- go get golang.org/x/tools/cmd/vet

script:
- go get -t -v ./...
- diff -u <(echo -n) <(gofmt -d -s .)
- diff -u <(echo -n) <(gofmt -d .)
- go tool vet .
- go test -v -race ./...
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 94b4359

Please sign in to comment.