Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

Fix sec vuln with list of claims #426

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# automatically generated by terraform - please do not edit here
* @form3tech-oss/Contributors-codeowners
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.DS_Store
bin
.idea/


9 changes: 4 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ script:
- go test -v ./...

go:
- 1.3
- 1.4
- 1.5
- 1.6
- 1.7
- 1.12
- 1.13
- 1.14
- 1.15
- tip
16 changes: 9 additions & 7 deletions claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Claims interface {
// https://tools.ietf.org/html/rfc7519#section-4.1
// See examples for how to use this with your own claim types
type StandardClaims struct {
Audience string `json:"aud,omitempty"`
Audience []string `json:"aud,omitempty"`
ExpiresAt int64 `json:"exp,omitempty"`
Id string `json:"jti,omitempty"`
IssuedAt int64 `json:"iat,omitempty"`
Expand Down Expand Up @@ -90,15 +90,17 @@ func (c *StandardClaims) VerifyNotBefore(cmp int64, req bool) bool {

// ----- helpers

func verifyAud(aud string, cmp string, required bool) bool {
if aud == "" {
func verifyAud(aud []string, cmp string, required bool) bool {
if len(aud) == 0 {
return !required
}
if subtle.ConstantTimeCompare([]byte(aud), []byte(cmp)) != 0 {
return true
} else {
return false

for _, a := range aud {
if subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 {
return true
}
}
return false
}

func verifyExp(exp int64, now int64, required bool) bool {
Expand Down
10 changes: 5 additions & 5 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package jwt_test

import (
"fmt"
"github.com/dgrijalva/jwt-go"
"github.com/form3tech-oss/jwt-go"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are this changes needed in example_test.go?
Functions should be named as Example - to have examples in godoc.
Also some external jwt-go is imported

"time"
)

Expand All @@ -11,7 +11,7 @@ import (
// to provide standard validation features. You can use it alone, but there's
// no way to retrieve other fields after parsing.
// See the CustomClaimsType example for intended usage.
func ExampleNewWithClaims_standardClaims() {
func NewWithClaims_standardClaims() {
mySigningKey := []byte("AllYourBase")

// Create the Claims
Expand All @@ -28,7 +28,7 @@ func ExampleNewWithClaims_standardClaims() {

// Example creating a token using a custom claims type. The StandardClaim is embedded
// in the custom type to allow for easy encoding, parsing and validation of standard claims.
func ExampleNewWithClaims_customClaimsType() {
func NewWithClaims_customClaimsType() {
mySigningKey := []byte("AllYourBase")

type MyCustomClaims struct {
Expand All @@ -53,7 +53,7 @@ func ExampleNewWithClaims_customClaimsType() {

// Example creating a token using a custom claims type. The StandardClaim is embedded
// in the custom type to allow for easy encoding, parsing and validation of standard claims.
func ExampleParseWithClaims_customClaimsType() {
func ParseWithClaims_customClaimsType() {
tokenString := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0In0.HE7fK0xOQwFEr4WDgRWj4teRPZ6i3GLwD5YCm6Pwu_c"

type MyCustomClaims struct {
Expand Down Expand Up @@ -87,7 +87,7 @@ func at(t time.Time, f func()) {
}

// An example of parsing the error types using bitfield checks
func ExampleParse_errorChecking() {
func Parse_errorChecking() {
// Token from another example. This token is expired
var tokenString = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0In0.HE7fK0xOQwFEr4WDgRWj4teRPZ6i3GLwD5YCm6Pwu_c"

Expand Down
10 changes: 9 additions & 1 deletion map_claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,15 @@ type MapClaims map[string]interface{}
// Compares the aud claim against cmp.
// If required is false, this method will return true if the value matches or is unset
func (m MapClaims) VerifyAudience(cmp string, req bool) bool {
aud, _ := m["aud"].(string)
aud, ok := m["aud"].([]string)
if !ok {
strAud, ok := m["aud"].(string)
aud = append(aud, strAud)
if !ok {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if should be before aud = append(aud, strAud)

return false
}
}

return verifyAud(aud, cmp, req)
}

Expand Down
71 changes: 71 additions & 0 deletions map_claims_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package jwt

import "testing"

func Test_mapClaims_list_aud(t *testing.T){
mapClaims := MapClaims{
"aud": []string{"foo"},
}
want := true
got := mapClaims.VerifyAudience("foo", true)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}
func Test_mapClaims_string_aud(t *testing.T){
mapClaims := MapClaims{
"aud": "foo",
}
want := true
got := mapClaims.VerifyAudience("foo", true)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}

func Test_mapClaims_list_aud_no_match(t *testing.T){
mapClaims := MapClaims{
"aud": []string{"bar"},
}
want := false
got := mapClaims.VerifyAudience("foo", true)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}
func Test_mapClaims_string_aud_fail(t *testing.T){
mapClaims := MapClaims{
"aud": "bar",
}
want := false
got := mapClaims.VerifyAudience("foo", true)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}

func Test_mapClaims_string_aud_no_claim(t *testing.T){
mapClaims := MapClaims{
}
want := false
got := mapClaims.VerifyAudience("foo", true)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}

func Test_mapClaims_string_aud_no_claim_not_required(t *testing.T){
mapClaims := MapClaims{
}
want := false
got := mapClaims.VerifyAudience("foo", false)

if want != got {
t.Fatalf("Failed to verify claims, wanted: %v got %v", want, got)
}
}