QuizGame: Administrative API module lets unauthenticated requests through
Closed, ResolvedPublicSecurity
Actions

Assigned To

Authored By

	ashley
	Feb 21 2022, 9:20 AM

Description

In 88754ee5b7c7f745f1172cf0238f8e65442e1bd0 (27 July 2013 (!)) I converted QuizGame to use an API module instead of the then-deprecated sajax_* functions provided by MediaWiki core to make the extension compatible with MediaWiki 1.21.
The old AJAX entry point, wfQuestionGameAdmin, which despite the name wasn't admin-only nor truly intended to be such as it handles flagging, did feature a rudimentary "is the request allowed?" check; but I'm not sure if it really provided any real security. Either way, when creating the API module, even this check was lost.

So right now all the API module really cares about is that:
1.) The request has a valid anti-CSRF token (new mw.Api().postWithEditToken( ... ) will take care of that)
2.) The request was POSTed
3.) The quizaction and id parameters are set and that the former corresponds to one of the actions handled by the module's switch() loop

As long as those requirements are met, an attacker can easily abuse the QuizGame administrative API to their nefarious purposes - quizadmin rights not needed!

Hilariously, though, the UI in /extensions/QuizGame/includes/specials/SpecialQuizGameHome.php does perform correct user right checks for the admin panel and whatnot and redirects unauthorized users to the main quiz game landing page.

Quick patch to add block and proper user rights checking to the QuizGame admin API (while allowing all users to flag quizzes):

diff --git a/includes/api/ApiQuizGame.php b/includes/api/ApiQuizGame.php
index 3236d7e..56d5a82 100644
--- a/includes/api/ApiQuizGame.php
+++ b/includes/api/ApiQuizGame.php
@@ -34,6 +34,21 @@ class ApiQuizGame extends ApiBase {
                // ApiBase's getDB() supports only slave connections, lame...
                $dbw = wfGetDB( DB_PRIMARY );
 
+               // Fail early if the user is sitewide blocked.
+               // (This snippet copied from MW core /includes/api/ApiTag.php)
+               $block = $user->getBlock();
+               if ( $block && $block->isSitewide() ) {
+                       $this->dieBlocked( $block );
+               }
+
+               // Allow non-quizadmins to use the flagging feature but require quizadmin
+               // rights for all other stuff
+               if ( $action !== 'flagItem' ) {
+                       if ( !$user->isAllowed( 'quizadmin' ) ) {
+                               $this->dieWithError( 'badaccess-group0' );
+                       }
+               }
+
                switch ( $action ) {
                        case 'unprotectItem':
                                $dbw->update(

Details

	Subject	Repo	Branch	Lines +/-
	[SECURITY] QuizGame admin API module should check for the 'quizadmin' user right for all actions except flagging	mediawiki/extensions/QuizGame	master	+13 -0

Customize query in gerrit

Related Objects

Mentioned In: T305209: Write and send supplementary release announcement for extensions and skins with security patches (1.35.7/1.37.3/1.38.2)
T297839: Write and send supplementary release announcement for extensions and skins with security patches (1.35.6/1.36.4/1.37.2)
Mentioned Here: rEQGA88754ee5b7c7: QuizGame: made compatible with MW 1.21.1.

Event Timeline

ashley created this task.Feb 21 2022, 9:20 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 21 2022, 9:20 AM

ashley claimed this task.Feb 21 2022, 9:21 AM

ashley added projects: Vuln-MissingAuthz, QuizGame, Social-Tools.

Adding @RhinosF1 of Miraheze per request since some Miraheze wikis use QuizGame.

RhinosF1 added a project: User-RhinosF1.Feb 21 2022, 9:08 PM

From 26cab2b8ca3fba27849c7453f48b7f9189e89fc9 Mon Sep 17 00:00:00 2001
From: www-data <www-data@miraheze.org>
Date: Mon, 21 Feb 2022 22:06:16 +0000
Subject: [SECURITY] QuizGame: Administrative API module lets
 unauthenticated requests through

---
 includes/api/ApiQuizGame.php | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/includes/api/ApiQuizGame.php b/includes/api/ApiQuizGame.php
index 3236d7e..081e64e 100644
--- a/includes/api/ApiQuizGame.php
+++ b/includes/api/ApiQuizGame.php
@@ -34,6 +34,21 @@ class ApiQuizGame extends ApiBase {
 		// ApiBase's getDB() supports only slave connections, lame...
 		$dbw = wfGetDB( DB_PRIMARY );
 
+               // Fail early if the user is sitewide blocked.
+               // (This snippet copied from MW core /includes/api/ApiTag.php)
+               $block = $user->getBlock();
+               if ( $block && $block->isSitewide() ) {
+                       $this->dieBlocked( $block );
+               }
+
+               // Allow non-quizadmins to use the flagging feature but require quizadmin
+               // rights for all other stuff
+               if ( $action !== 'flagItem' ) {
+                       if ( !$user->isAllowed( 'quizadmin' ) ) {
+                               $this->dieWithError( 'badaccess-group0' );
+                       }
+               }
+
 		switch ( $action ) {
 			case 'unprotectItem':
 				$dbw->update(
-- 
2.30.2

didn't apply for me, ended up working with sudo -u www-data git apply --ignore-space-change --ignore-whitespace ~/quizgame.patch

That's the new patch file

sbassett edited projects, added SecTeam-Processed; removed Security-Team.Feb 22 2022, 4:33 PM

sbassett mentioned this in T297839: Write and send supplementary release announcement for extensions and skins with security patches (1.35.6/1.36.4/1.37.2).

The patch looks correct to me, +2. One nit:

+               if ( $action !== 'flagItem' ) {
+                       if ( !$user->isAllowed( 'quizadmin' ) ) {
+                               $this->dieWithError( 'badaccess-group0' );
+                       }
+               }

These two if blocks can be merged: if ( $action !== 'flagItem' && !$user->isAllowed( 'quizadmin' ) ).

(Aso in the future it's better to post patches as attachments generated with git format-patch HEAD~1, so that any weird whitespace, etc. are preserved.)

Also, also, I noticed that SpecialQuizGameHome is checking for *any* block, not a sitewide block. That can be fixed publicly, I just looked because you mentioned it implemented the checks correctly.

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 13 2022, 6:38 PM

Reedy changed the edit policy from "Custom Policy" to "All Users".

• MZMcBride subscribed.Mar 14 2022, 4:46 AM

Finally closing this as the patch was merged about a month ago (oops). @Legoktm's note regarding Special:QuizGameHome is worth investigating, but that's a separate matter altogether. (Social-Tools basically have no knowledge and thus no support for the concept of partial blocks; you're either blocked or not, social tools don't currently care about how partial the block is.)

Maintenance_bot moved this task from Radar to Done on the User-RhinosF1 board.Mar 28 2022, 5:15 PM

sbassett mentioned this in T305209: Write and send supplementary release announcement for extensions and skins with security patches (1.35.7/1.37.3/1.38.2).Apr 18 2022, 4:12 PM

QuizGame: Administrative API module lets unauthenticated requests throughClosed, ResolvedPublicSecurityActions

Description

Details

Related Objects

Event Timeline

QuizGame: Administrative API module lets unauthenticated requests through
Closed, ResolvedPublicSecurity
Actions