CSteipp has uploaded a new change for review.
https://gerrit.wikimedia.org/r/74643
Change subject: Prevent empty secrets
......................................................................
Prevent empty secrets
Add sanity checks to make sure secrets exist before checking the
signatures.
Also some whitespace and formatting cleanup.
Change-Id: Ib821c9baefa5a51e6809285f47a88cbf8b843e23
---
M backend/MWOAuthServer.php
M frontend/specialpages/SpecialMWOAuth.php
M lib/OAuth.php
3 files changed, 30 insertions(+), 7 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/OAuth
refs/changes/43/74643/1
diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php
index 9b7fd7e..4cdfc47 100644
--- a/backend/MWOAuthServer.php
+++ b/backend/MWOAuthServer.php
@@ -14,6 +14,11 @@
$consumer = $this->get_consumer( $request );
+ // Consumer must have a key for us to verify
+ if ( !$consumer->get( 'secretKey' ) && !$consumer->get(
'rsaKey' ) ) {
+ throw new MWOAuthException( 'invalid-consumer' );
+ }
+
$this->checkSourceIP( $consumer, $request );
// no token required for the initial token request
@@ -41,10 +46,20 @@
$consumer = $this->get_consumer( $request );
+ // Consumer must have a key for us to verify
+ if ( !$consumer->get( 'secretKey' ) && !$consumer->get(
'rsaKey' ) ) {
+ throw new MWOAuthException( 'invalid-consumer' );
+ }
+
$this->checkSourceIP( $consumer, $request );
// requires authorized request token
- $token = $this->get_token( $request, $consumer, "request" );
+ $token = $this->get_token( $request, $consumer, 'request' );
+
+ if ( !$token->secret ) {
+ // This token has a blank secret.. something is wrong
+ throw new MWOAuthException( 'bad-token' );
+ }
$this->check_signature( $request, $consumer, $token );
@@ -67,6 +82,11 @@
$restrictions = $consumer->get( 'restrictions' );
$requestIP = $request->getSourceIP();
+ // If there are no restrictions, don't check anything
+ if ( !$restrictions ) {
+ return true;
+ }
+
foreach ( $restrictions['IPAddresses'] as $range ) {
if ( IP::isInRange( $requestIP, $range ) ) {
return true;
diff --git a/frontend/specialpages/SpecialMWOAuth.php
b/frontend/specialpages/SpecialMWOAuth.php
index 523277b..6809a5d 100644
--- a/frontend/specialpages/SpecialMWOAuth.php
+++ b/frontend/specialpages/SpecialMWOAuth.php
@@ -36,6 +36,7 @@
if ( !$requestToken || !$consumerKey ) {
throw new MWOAuthException(
'mwoauth-bad-request' );
}
+ // TODO? Test that $requestToken exists
in memcache
if ( $mwUser->isAnon() ) {
//redirect to login
@@ -94,7 +95,7 @@
break;
case 'token':
$OAuthRequest =
MWOAuthRequest::fromRequest( $request );
- wfDebugLog( 'OAuth', "/token:
'{$OAuthRequest->getConsumerKey()}' getting temporary credentials" );
+ wfDebugLog( 'OAuth', "/token:
'{$OAuthRequest->get_parameter( 'oauth_consumer_key' )}' getting temporary
credentials" );
$token =
$oauthServer->fetch_access_token( $OAuthRequest );
$this->returnToken( $token, $format );
diff --git a/lib/OAuth.php b/lib/OAuth.php
index f281597..3e794ee 100644
--- a/lib/OAuth.php
+++ b/lib/OAuth.php
@@ -662,9 +662,9 @@
? $request->get_parameter( 'oauth_token' )
: NULL;
- $token = $this->data_store->lookup_token(
+ $token = $this->data_store->lookup_token(
$consumer, $token_type, $token_field
- );
+ );
if ( !$token ) {
throw new OAuthException( "Invalid $token_type token:
$token_field" );
}
@@ -706,10 +706,11 @@
* check that the timestamp is new enough
*/
private function check_timestamp( $timestamp ) {
- if( ! $timestamp )
+ if( !$timestamp ) {
throw new OAuthException(
'Missing timestamp parameter. The parameter is
required'
);
+ }
// verify that timestamp is recentish
$now = time();
@@ -724,13 +725,14 @@
* check that the nonce is not repeated
*/
private function check_nonce( $consumer, $token, $nonce, $timestamp ) {
- if( ! $nonce )
+ if( !$nonce ) {
throw new OAuthException(
'Missing nonce parameter. The parameter is
required'
);
+ }
// verify that the nonce is uniqueish
- $found = $this->data_store->lookup_nonce(
+ $found = $this->data_store->lookup_nonce(
$consumer,
$token,
$nonce,
--
To view, visit https://gerrit.wikimedia.org/r/74643
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib821c9baefa5a51e6809285f47a88cbf8b843e23
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/OAuth
Gerrit-Branch: master
Gerrit-Owner: CSteipp <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
