CVE-2020-15223 (Go, published 2020-09-24T17:15Z)
Description: value: In ORY Fosite (the security first OAuth2 & OpenID Connect framework for Go) before version 0.34.0, the `TokenRevocationHandler` ignores errors coming from the storage. This can lead to unexpected 200 status codes indicating successful revocation while the token is still valid. Whether an attacker can use this for her advantage depends on the ability to trigger errors in the store. This is fixed in version 0.34.0
Severity: MEDIUM (CVSS v3 base score: 8)
Weakness: CWE-754 \u2014 Improper Check for Unusual or Exceptional Conditions
The product does not check or incorrectly checks for unusual or exceptional conditions that are not expected to occur frequently during day to day operation of the product.
Repository: https://github.com/ory/fosite
Files changed:
.golangci.yml
access_error.go
access_write.go
authorize_error.go
errors.go
errors_test.go
go_mod_indirect_pins.go
handler/oauth2/revocation.go
handler/oauth2/revocation_test.go
handler/openid/helper.go
revoke_handler.go
token/hmac/hmacsha.go
Commit message: Merge pull request from GHSA-7mqr-2v3q-v2wm

BREAKING CHANGE: `fosite.ErrRevocationClientMismatch` was removed because it is not part of [RFC 6749](https://tools.ietf.org/html/rfc6749#section-5.2). Instead, `fosite.ErrUnauthorizedClient` will be returned when calling `RevokeToken` with an OAuth2 Client which does not match the Access or Refresh Token to be revoked.

Vulnerable code:
    - errcheck
	rw.Write(js)
	rw.Write(js)
		rw.Write(js)
	ErrRevocationClientMismatch = &RFC6749Error{
		Name:        errRevocationClientMismatchName,
		Description: "Token was not issued to the client making the revocation request",
		Code:        http.StatusBadRequest,
	}
func TestAddDebug(t *testing.T) {
	err := ErrRevocationClientMismatch.WithDebug("debug")
	assert.NotEqual(t, err, ErrRevocationClientMismatch)
	assert.Empty(t, ErrRevocationClientMismatch.Debug)
	assert.NotEmpty(t, err.Debug)
}

	var err error
	if ar, err = discoveryFuncs[0](); err != nil {
		ar, err = discoveryFuncs[1]()
	if err != nil {
		return err
		return errors.WithStack(fosite.ErrRevocationClientMismatch)
	r.TokenRevocationStorage.RevokeRefreshToken(ctx, requestID)
	r.TokenRevocationStorage.RevokeAccessToken(ctx, requestID)
	return nil
			expectErr:   fosite.ErrRevocationClientMismatch,
			description: "should fail - refresh token discovery first; both tokens not found",
			expectErr:   fosite.ErrNotFound,
			description: "should fail - access token discovery first; both tokens not found",
			expectErr:   fosite.ErrNotFound,
		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
	hash.Write(buffer.Bytes())
		rw.Write(js)
		rw.Write(js)
	h.Write(data)

Fixed code:
	// ignoring the error because the connection is broken when it happens
	_, _ = rw.Write(js)
	_, _ = rw.Write(js)
		_, _ = rw.Write(js)

	var err1, err2 error
	if ar, err1 = discoveryFuncs[0](); err1 != nil {
		ar, err2 = discoveryFuncs[1]()
	// err2 can only be not nil if first err1 was not nil
	if err2 != nil {
		return storeErrorsToRevocationError(err1, err2)
		return errors.WithStack(fosite.ErrUnauthorizedClient)
	err1 = r.TokenRevocationStorage.RevokeRefreshToken(ctx, requestID)
	err2 = r.TokenRevocationStorage.RevokeAccessToken(ctx, requestID)
	return storeErrorsToRevocationError(err1, err2)
}

func storeErrorsToRevocationError(err1, err2 error) error {
	// both errors are 404 or nil <=> the token is revoked
	if (errors.Is(err1, fosite.ErrNotFound) || err1 == nil) && (errors.Is(err2, fosite.ErrNotFound) || err2 == nil) {
		return nil
	}

	// there was an unexpected error => the token may still exist and the client should retry later
	return errors.WithStack(fosite.ErrTemporarilyUnavailable)
			expectErr:   fosite.ErrUnauthorizedClient,
			description: "should pass - refresh token discovery first; both tokens not found",
			expectErr:   nil,
			description: "should pass - access token discovery first; both tokens not found",
			expectErr:   nil,
		{
			description: "should fail - store error for access token get",
			expectErr:   fosite.ErrTemporarilyUnavailable,
			client:      &fosite.DefaultClient{ID: "bar"},
			mock: func() {
				token = "foo"
				tokenType = fosite.AccessToken
				atStrat.EXPECT().AccessTokenSignature(token)
				store.EXPECT().GetAccessTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("random error"))

				rtStrat.EXPECT().RefreshTokenSignature(token)
				store.EXPECT().GetRefreshTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fosite.ErrNotFound)
			},
		},
		{
			description: "should fail - store error for refresh token get",
			expectErr:   fosite.ErrTemporarilyUnavailable,
			client:      &fosite.DefaultClient{ID: "bar"},
			mock: func() {
				token = "foo"
				tokenType = fosite.RefreshToken
				atStrat.EXPECT().AccessTokenSignature(token)
				store.EXPECT().GetAccessTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fosite.ErrNotFound)

				rtStrat.EXPECT().RefreshTokenSignature(token)
				store.EXPECT().GetRefreshTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("random error"))
			},
		},
		{
			description: "should fail - store error for access token revoke",
			expectErr:   fosite.ErrTemporarilyUnavailable,
			client:      &fosite.DefaultClient{ID: "bar"},
			mock: func() {
				token = "foo"
				tokenType = fosite.AccessToken
				atStrat.EXPECT().AccessTokenSignature(token)
				store.EXPECT().GetAccessTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(ar, nil)

				ar.EXPECT().GetID()
				ar.EXPECT().GetClient().Return(&fosite.DefaultClient{ID: "bar"})
				store.EXPECT().RevokeRefreshToken(gomock.Any(), gomock.Any()).Return(fosite.ErrNotFound)
				store.EXPECT().RevokeAccessToken(gomock.Any(), gomock.Any()).Return(fmt.Errorf("random error"))
			},
		},
		{
			description: "should fail - store error for refresh token revoke",
			expectErr:   fosite.ErrTemporarilyUnavailable,
			client:      &fosite.DefaultClient{ID: "bar"},
			mock: func() {
				token = "foo"
				tokenType = fosite.RefreshToken
				rtStrat.EXPECT().RefreshTokenSignature(token)
				store.EXPECT().GetRefreshTokenSession(gomock.Any(), gomock.Any(), gomock.Any()).Return(ar, nil)

				ar.EXPECT().GetID()
				ar.EXPECT().GetClient().Return(&fosite.DefaultClient{ID: "bar"})
				store.EXPECT().RevokeRefreshToken(gomock.Any(), gomock.Any()).Return(fmt.Errorf("random error"))
				store.EXPECT().RevokeAccessToken(gomock.Any(), gomock.Any()).Return(fosite.ErrNotFound)
			},
		},
		t.Run(fmt.Sprintf("case=%d/description=%s", k, c.description), func(t *testing.T) {
	// sha256.digest.Write() always returns nil for err, the panic should never happen
	_, err := hash.Write(buffer.Bytes())
	if err != nil {
		panic(err)
	}
		_, _ = rw.Write(js)
		_, _ = rw.Write(js)
	// sha512.digest.Write() always returns nil for err, the panic should never happen
	_, err := h.Write(data)
	if err != nil {
		panic(err)
	}