Page MenuHomePhabricator

OAuth allows to view accepted consumers of other users
Closed, ResolvedPublic

Description

[[Special:MWOAuthManageMyGrants]] does not check whether the ID passed as an URL argument belongs to the current user, and allows the user to view any existing accepted consumer settings (see e.g. the URL linked above). Fortunately, MWOAuthConsumerAcceptanceSubmitControl seems to check the current user correctly, so that attempts to modify the consumer should fail. So that while this is a security vulnerabilty, it is not a severe one.

I guess something like this could suffice?

  • a/frontend/specialpages/SpecialMWOAuthManageMyGrants.php

+++ b/frontend/specialpages/SpecialMWOAuthManageMyGrants.php
@@ -102,10 +102,14 @@ class SpecialMWOAuthManageMyGrants extends UnlistedSpecialPage {

		$user = $this->getUser();
		$lang = $this->getLanguage();
		$db = MWOAuthUtils::getCentralDB( DB_SLAVE );

+ $centralUserId = MWOAuthUtils::getCentralIdFromLocalUser( $user );
+ if ( !$centralUserId ) { // sanity
+ throw new PermissionsError();
+ }

		$cmra = MWOAuthDAOAccessControl::wrap(
			MWOAuthConsumerAcceptance::newFromId( $db, $acceptanceId ), $this->getContext() );
  • if ( !$cmra ) {

+ if ( !$cmra || $cmra->get( 'userId' ) !== $centralUserId ) {

			$this->getOutput()->addHtml( $this->msg( 'mwoauth-invalid-access-token' )->escaped() );
			return;
		}

Version: unspecified
Severity: major
URL: https://www.mediawiki.org/wiki/Special:MWOAuthManageMyGrants/manage/2

Details

Reference
bz53423

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:54 AM
bzimport set Reference to bz53423.

Yikes. Aaron, could you test the patch?

Change 81400 had a related patch set uploaded by Aaron Schulz:
Do not show grant change form if the user cannot submit it.

https://gerrit.wikimedia.org/r/81400

Change 81400 merged by CSteipp:
Do not show grant change form if the user cannot submit it.

https://gerrit.wikimedia.org/r/81400