Page MenuHomePhabricator
Files F4561092

0001-SECURITY-Fix-bugs-with-handling-of-missing-request-p.patch
Tgr (Gergő Tisza)
Actions

Authored By
Tgr
Oct 5 2016, 8:12 PM
Size
6 KB
Referenced Files
None
Subscribers
None

0001-SECURITY-Fix-bugs-with-handling-of-missing-request-p.patch
View Options

From 41417a401a15df1b6a7993a4669e0ec95aceda5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C5=91=20Tisza?= <gtisza@wikimedia.org>
Date: Wed, 5 Oct 2016 19:35:31 +0000
Subject: [PATCH] SECURITY: Fix bugs with handling of missing request
parameters
* make MWOAuthDAO methods type-safe in the face of MySQL non-strict
mode weirdness
* throw an exception when cancelled with an invalid consumer key
(should not happen under normal circumstances as the authorization
dialog won't display)
* fix phpdoc so it's easier to follow in an IDE what happens
Bug: T147414
Change-Id: Ibb938ccb9bfae6c52444f7676dc475f6a3024cd8
---
backend/MWOAuthConsumer.php | 8 ++++++--
backend/MWOAuthConsumerAcceptance.php | 7 ++++---
backend/MWOAuthDAO.php | 2 +-
backend/MWOAuthDataStore.php | 6 +++---
backend/MWOAuthServer.php | 5 ++++-
frontend/specialpages/SpecialMWOAuth.php | 3 +++
6 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/backend/MWOAuthConsumer.php b/backend/MWOAuthConsumer.php
index 2289888..d63fde3 100644
--- a/backend/MWOAuthConsumer.php
+++ b/backend/MWOAuthConsumer.php
@@ -163,7 +163,7 @@ class MWOAuthConsumer extends MWOAuthDAO {
public static function newFromKey( \DBConnRef $db, $key, $flags = 0 ) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
- array( 'oarc_consumer_key' => $key ),
+ array( 'oarc_consumer_key' => (string)$key ),
__METHOD__,
( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) : array()
);
@@ -190,7 +190,11 @@ class MWOAuthConsumer extends MWOAuthDAO {
) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
- array( 'oarc_name' => $name, 'oarc_version' => $version, 'oarc_user_id' => $userId ),
+ array(
+ 'oarc_name' => (string)$name,
+ 'oarc_version' => (string)$version,
+ 'oarc_user_id' => (int)$userId
+ ),
__METHOD__,
( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) : array()
);
diff --git a/backend/MWOAuthConsumerAcceptance.php b/backend/MWOAuthConsumerAcceptance.php
index 1851278..474f992 100644
--- a/backend/MWOAuthConsumerAcceptance.php
+++ b/backend/MWOAuthConsumerAcceptance.php
@@ -81,7 +81,7 @@ class MWOAuthConsumerAcceptance extends MWOAuthDAO {
public static function newFromToken( \DBConnRef $db, $token, $flags = 0 ) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
- array( 'oaac_access_token' => $token ),
+ array( 'oaac_access_token' => (string)$token ),
__METHOD__,
( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) : array()
);
@@ -108,9 +108,10 @@ class MWOAuthConsumerAcceptance extends MWOAuthDAO {
) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
- array( 'oaac_user_id' => $userId,
+ array(
+ 'oaac_user_id' => (int)$userId,
'oaac_consumer_id' => $consumer->get( 'id' ),
- 'oaac_wiki' => $wiki
+ 'oaac_wiki' => (string)$wiki
),
__METHOD__,
( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) : array()
diff --git a/backend/MWOAuthDAO.php b/backend/MWOAuthDAO.php
index 6b3d76f..62683e9 100644
--- a/backend/MWOAuthDAO.php
+++ b/backend/MWOAuthDAO.php
@@ -75,7 +75,7 @@ abstract class MWOAuthDAO implements \IDBAccessObject {
final public static function newFromId( \DBConnRef $db, $id, $flags = 0 ) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
- array( static::getIdColumn() => $id ),
+ array( static::getIdColumn() => (int)$id ),
__METHOD__,
( $flags & self::READ_LOCKING ) ? array( 'FOR UPDATE' ) : array()
);
diff --git a/backend/MWOAuthDataStore.php b/backend/MWOAuthDataStore.php
index 2dd2d81..76de10f 100644
--- a/backend/MWOAuthDataStore.php
+++ b/backend/MWOAuthDataStore.php
@@ -149,7 +149,7 @@ class MWOAuthDataStore extends OAuthDataStore {
* Return a consumer key associated with the given request token.
*
* @param MWOAuthToken $requestToken the request token
- * @return String the consumer key
+ * @return string|false the consumer key or false if nothing is stored for the request token
*/
public function getConsumerKey( $requestToken ) {
$cacheKey = MWOAuthUtils::getCacheKey( 'consumer', 'request', $requestToken );
@@ -163,10 +163,10 @@ class MWOAuthDataStore extends OAuthDataStore {
* A stored callback URL parameter is deleted from the cache once read for the first
* time.
*
- * @param string the consumer key
+ * @param string $consumerKey the consumer key
* @param string $requestKey original request key from /initiate
* @throws MWOAuthException
- * @return String the stored callback URL parameter
+ * @return string|false the stored callback URL parameter
*/
public function getCallbackUrl( $consumerKey, $requestKey ) {
$cacheKey = MWOAuthUtils::getCacheKey( 'callback', $consumerKey, 'request', $requestKey );
diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php
index fa178cc..e643ebf 100644
--- a/backend/MWOAuthServer.php
+++ b/backend/MWOAuthServer.php
@@ -3,11 +3,14 @@
namespace MediaWiki\Extensions\OAuth;
class MWOAuthServer extends OAuthServer {
+ /** @var MWOAuthDataStore */
+ protected $data_store;
+
/**
* Return a consumer key associated with the given request token.
*
* @param MWOAuthToken $requestToken the request token
- * @return String the consumer key
+ * @return string|false the consumer key or false if nothing is stored for the request token
*/
public function getConsumerKey( $requestToken ) {
return $this->data_store->getConsumerKey( $requestToken );
diff --git a/frontend/specialpages/SpecialMWOAuth.php b/frontend/specialpages/SpecialMWOAuth.php
index 0397987..58da23c 100644
--- a/frontend/specialpages/SpecialMWOAuth.php
+++ b/frontend/specialpages/SpecialMWOAuth.php
@@ -218,6 +218,9 @@ class SpecialMWOAuth extends \UnlistedSpecialPage {
MWOAuthConsumer::newFromKey( $dbr, $consumerKey ),
$this->getContext()
);
+ if ( !$cmr ) {
+ throw new \ErrorPageError( 'mwoauth-error', 'mwoauth-invalid-consumer-key' );
+ }
$this->getOutput()->addSubtitle( $this->msg( 'mwoauth-desc' )->escaped() );
$this->getOutput()->addWikiMsg(
--
1.9.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
4036014
Default Alt Text
0001-SECURITY-Fix-bugs-with-handling-of-missing-request-p.patch (6 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL