Doorkeeper: Token revocation uses wrong authorization method

[jclem]

The OAuth 2.0 Token Revocation spec states that when revoking a token, "the client also includes its authentication credentials as described in Section 2.3. of [RFC6749]".

The spec's example request for revocation looks like this:

POST /revoke HTTP/1.1
     Host: server.example.com
     Content-Type: application/x-www-form-urlencoded
     Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW

     token=45ghiukldjahdnhzdauz&token_type_hint=refresh_token

Note the HTTP Basic authentication, not Bearer. The spec goes on to say that the authorization server "first validates the client credentials (in case of a confidential client) and then verifies whether the token was issued to the client making the revocation request. If this validation fails, the request is refused and the client is informed of the error by the authorization server as described below".

It appears as though Doorkeeper instead authorizes the request using HTTP Bearer with an access token, even though a code comment seems to imply otherwise.

[tute]

Thank you for reporting. This has been reported by @f3ndot too, and we are now working on it. Thanks!

[jclem]

You're welcome, and thank you for maintaining Doorkeeper!

[f3ndot]

This issue has been assigned CVE-2016-6582 as properly authenticated confidential clients will not have their token revoked, with no error from Doorkeeper to indicate otherwise.

@tute and myself have been coordinating a fix as this also affects public, non-confidential clients as well.

[f3ndot]

The security implication is: clients who "log out" a user expect to have the corresponding access & refresh tokens revoked, preventing an attacker who may have already hijacked the session from continuing to impersonate the victim. Because of this bug, this is not the case. As far as MITRE and OWASP are concerned this counts as broken authentication design.

Unless I'm mistaken, all clients (public or confidential) that send well-formed, RFC 7009 compliant requests are affected by this bug. All versions of Doorkeeper that have supported this feature (1.2.0 and later) are affected by this bug.

[tute]

Thank you both!

[f3ndot]

Added an advisory entry to https://github.com/rubysec/ruby-advisory-db/pull/268

[f3ndot]

Findings published: http://seclists.org/oss-sec/2016/q3/332