Merge pull request dgrijalva#429 from Waterdrips/waterdrips-fix-cve

Fix security issue with aud validation

.gitignore

@@ -1,4 +1,4 @@
 .DS_Store
 bin
-
+.idea/
 

claims.go

@@ -61,7 +61,7 @@ func (c StandardClaims) Valid() error {
 // Compares the aud claim against cmp.
 // If required is false, this method will return true if the value matches or is unset
 func (c *StandardClaims) VerifyAudience(cmp string, req bool) bool {
-	return verifyAud(c.Audience, cmp, req)
+	return verifyAud([]string{c.Audience}, cmp, req)
 }
 
 // Compares the exp claim against cmp.
@@ -90,15 +90,27 @@ 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
+	// use a var here to keep constant time compare when looping over a number of claims
+	result := false
+
+	var stringClaims string
+	for _, a := range aud {
+		if subtle.ConstantTimeCompare([]byte(a), []byte(cmp)) != 0 {
+			result = true
+		}
+		stringClaims = stringClaims + a
 	}
+
+	// case where "" is sent in one or many aud claims
+	if len(stringClaims) == 0 {
+		return !required
+	}
+
+	return result
 }
 
 func verifyExp(exp int64, now int64, required bool) bool {

map_claims.go

@@ -10,10 +10,24 @@ import (
 // This is the default claims type if you don't supply one
 type MapClaims map[string]interface{}
 
-// Compares the aud claim against cmp.
+// VerifyAudience 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)
+	var aud []string
+	switch v := m["aud"].(type) {
+	case string:
+		aud = append(aud, v)
+	case []string:
+		aud = v
+	case []interface{}:
+		for _, a := range v {
+			vs, ok := a.(string)
+			if !ok {
+				return false
+			}
+			aud = append(aud, vs)
+		}
+	}
 	return verifyAud(aud, cmp, req)
 }
 

map_claims_test.go

@@ -0,0 +1,68 @@
+package jwt
+
+import (
+	"testing"
+)
+
+func TestVerifyAud(t *testing.T) {
+	var nilInterface interface{}
+	var nilListInterface []interface{}
+	var intListInterface interface{} = []int{1,2,3}
+	type test struct{
+		Name string
+		MapClaims MapClaims
+		Expected bool
+		Comparison string
+		Required bool
+	}
+	tests := []test{
+		// Matching Claim in aud
+		// Required = true
+		{ Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true , Required: true, Comparison: "example.com"},
+		{ Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
+
+		// Required = false
+		{ Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true , Required: false, Comparison: "example.com"},
+		{ Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true , Required: false, Comparison: "example.com"},
+		{ Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true , Required: false, Comparison: "example.com"},
+		{ Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true , Required: false, Comparison: "example.com"},
+
+		{ Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: "example.com"},
+		{ Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: "example.com"},
+
+		// Non-Matching Claim in aud
+		// Required = true
+		{ Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
+
+		// Required = false
+		{ Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
+
+		// []interface{}
+		{ Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: "example.com"},
+		{ Name: "[]interface{} Aud wit match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
+		{ Name: "[]interface{} Aud wit match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
+		{ Name: "[]interface{} Aud int wit match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: "example.com"},
+
+
+		// interface{}
+		{ Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: "example.com"},
+
+	}
+
+
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			got := test.MapClaims.VerifyAudience(test.Comparison, test.Required)
+
+			if got != test.Expected {
+				t.Errorf("Expected %v, got %v", test.Expected, got)
+			}
+		})
+	}
+}