diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be9736e..bfce323a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - [GHSA-f24x-5g9q-753f](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-f24x-5g9q-753f) fix: clear session cookie at beginning of signinpage handler (@fnoehWM / @bella-WI / @tuunit) - [GHSA-5hvv-m4w4-gf6v](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-5hvv-m4w4-gf6v) fix: health check user-agent authentication bypass (@tuunit) - [GHSA-7x63-xv5r-3p2x](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-7x63-xv5r-3p2x) fix: authentication bypass via X-Forwarded-Uri header spoofing (@tuunit) +- [GHSA-c5c4-8r6x-56w3](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-c5c4-8r6x-56w3) fix: email validation bypass via malformed multi-@ email claims (@tuunit) # V7.15.1 diff --git a/oauthproxy_test.go b/oauthproxy_test.go index df5912a0..fc9c34bd 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -3486,6 +3486,24 @@ func TestAuthOnlyAllowedEmailDomains(t *testing.T) { querystring: "?allowed_email_domains=a.b.c.example.com,*.c.example.com", expectedStatusCode: http.StatusAccepted, }, + { + name: "UserWithMultipleAtSignsExactDomain", + email: "attacker@evil.com@example.com", + querystring: "?allowed_email_domains=example.com", + expectedStatusCode: http.StatusForbidden, + }, + { + name: "UserWithMultipleAtSignsWildcardDomain", + email: "attacker@evil.com@foo.example.com", + querystring: "?allowed_email_domains=*.example.com", + expectedStatusCode: http.StatusForbidden, + }, + { + name: "UserWithMultipleAtSignsDotPrefixedDomain", + email: "attacker@evil.com@foo.example.com", + querystring: "?allowed_email_domains=.example.com", + expectedStatusCode: http.StatusForbidden, + }, } for _, tc := range testCases { diff --git a/validator.go b/validator.go index 587ef857..d03157a5 100644 --- a/validator.go +++ b/validator.go @@ -110,6 +110,10 @@ func NewValidator(domains []string, usersFile string) func(string) bool { // isEmailValidWithDomains checks if the authenticated email is validated against the provided domain func isEmailValidWithDomains(email string, allowedDomains []string) bool { + if strings.Count(email, "@") != 1 { + return false + } + for _, domain := range allowedDomains { // allow if the domain is perfect suffix match with the email if strings.HasSuffix(email, "@"+domain) { @@ -119,7 +123,6 @@ func isEmailValidWithDomains(email string, allowedDomains []string) bool { // allow if the domain is prefixed with . or *. and // the last element (split on @) has the suffix as the domain atoms := strings.Split(email, "@") - if (strings.HasPrefix(domain, ".") && strings.HasSuffix(atoms[len(atoms)-1], domain)) || (strings.HasPrefix(domain, "*.") && strings.HasSuffix(atoms[len(atoms)-1], domain[1:])) { return true diff --git a/validator_test.go b/validator_test.go index 976c0d7d..c406a732 100644 --- a/validator_test.go +++ b/validator_test.go @@ -404,6 +404,27 @@ func TestValidatorCases(t *testing.T) { allowedDomains: []string{"*.company.com"}, expectedAuthZ: false, }, + { + name: "CheckThatTwoAtSignsIsInvalid", + email: "attacker@evil.com@company.com", + allowedEmails: []string(nil), + allowedDomains: []string{"company.com"}, + expectedAuthZ: false, + }, + { + name: "CheckThatTwoAtSignsIsInvalidEvenWithDotPrefix", + email: "attacker@evil.com@company.com", + allowedEmails: []string(nil), + allowedDomains: []string{".company.com"}, + expectedAuthZ: false, + }, + { + name: "CheckThatTwoAtSignsIsInvalidEvenWithWildcardPrefix", + email: "attacker@evil.com@foo.company.com", + allowedEmails: []string(nil), + allowedDomains: []string{"*.company.com"}, + expectedAuthZ: false, + }, } for _, tc := range testCases {