Page MenuHomePhabricator

API requests don't get validated if signed by the correct OAuth consumer
Closed, ResolvedPublic


API requests with OAuth authentication don't seem to get checked if they are signed with the correct consumer token, a consumer secret from a different consumer also works.

As a test case:

  • SiticTestUser2 authorizes consumer 571d07e4e3a8209ee5e83f603e8c4f61 (owned by Sitic, this is the only connected app for SiticTestUser2)
  • the negotiated client token are key = 'cf88403442d8b6c025e7aa5693452903', secret = 'a392f8b5913af3cd97dcdef6ed120f0c2375c07d'
  • the attacker SiticTestUser steals that client token (from SiticTestUser2)
  • SiticTestUser created the new OAuth consumer 0c3c967b69938b175eb595ace1e2cb2f with consumer token key='0c3c967b69938b175eb595ace1e2cb2f', secret='f58062a99780afbb5101534fbc668adb366ae333' (doesn't need to get approved by OAuth admins)
  • SiticTestUser makes an API request with the client token from SiticTestUser2 and his new OAuth consumer token -> this should fail, but it doesn't

Test code:

import requests
from requests_oauthlib import OAuth1

# access token for SiticTestUser2 obtained from OAuth consumer 571d07e4e3a8209ee5e83f603e8c4f61 (approved consumer, owned by Sitic)
access_token =  {'key': 'cf88403442d8b6c025e7aa5693452903',
                 'secret': 'a392f8b5913af3cd97dcdef6ed120f0c2375c07d'}

# consumer token for OAuth consumer 0c3c967b69938b175eb595ace1e2cb2f
# This consumer is _not_ authorized by SiticTestUser2 and also is not an approved consumer by OAuth Admins (owned by SiticTestUser)
consumer_token = {'key': '0c3c967b69938b175eb595ace1e2cb2f',
                  'secret': 'f58062a99780afbb5101534fbc668adb366ae333'}

auth1 = OAuth1(consumer_token['key'],
response = requests.get(
        'action': "query",
        'list': "watchlist",
        'wlprop': "title",
        'format': "json"
print response.json()
# Should give error mwoauth-invalid-authorization, but shows watchlist for SiticTestUser2:
# {u'query': {u'watchlist': [{u'ns': 0, u'type': u'edit', u'title': u'Sandbox'}, {u'ns': 2, u'type': u'log', u'title': u'User:SiticTestUser2'}]}}

In summary with only the client/access token for a user I can pretend to be that consumer and make API requests in his stead for said user. Because of T103022 IP restrictions by original consumer don't work. As a note here the API permissions (“grants”) from the original authorized consumer seem to apply, not from the consumer of the attacker (I got permissiondenied when trying to make an edit).

I stumbled upon this due to the tools outage.

Event Timeline

Maniphest changed the visibility from "Public (No Login Required)" to "Custom Policy".Jun 18 2015, 10:08 PM
Maniphest changed the edit policy from "All Users" to "Custom Policy".
Sitic updated the task description. (Show Details)
Sitic changed Security from None to Software security bug.
Sitic edited subscribers, added: Sitic; removed: Aklapper.
csteipp subscribed.

I think there is a check missing in MWOAuthDataStore::lookup_token(). For access tokens, we lookup the consumer by the access token, and don't confirm that the id's match.

This requires that you have the access token for a different consumer's users, which should be a rare occurrence.

Looks sane. Haven't tested.

@Sitic, do you have a local OAuth instance that you can test this patch and verify that it fixes the issue?

@Sitic, do you have a local OAuth instance that you can test this patch and verify that it fixes the issue?

Tested locally, fixes the issue (getting The authorization headers in your request are not valid: No approved grant was found for that authorization token error info now).

@mmodell, can you deploy

and confirm here when it's on the cluster?

csteipp moved this task from In Progress to Waiting on the Security-Team board.
csteipp moved this task from Waiting to In Progress on the Security-Team board.
demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 16 2015, 10:35 PM
demon changed the edit policy from "Custom Policy" to "All Users".
demon changed Security from Software security bug to None.

Change 247026 had a related patch set uploaded (by Gergő Tisza):
Handle empty return from MWOAuthDataStore::lookup_consumer

Change 247026 merged by jenkins-bot:
Handle empty return from MWOAuthDataStore::lookup_consumer