T312820.patch
mmartorana (manfredi martorana)
Actions

Authored By

	mmartorana
	Sep 15 2022, 5:02 PM

Size

2 KB

Referenced Files

None

Subscribers

None

T312820.patch
View Options

	From 5b9942d04cce56a7c5a2e96a9705549b092d0c61 Mon Sep 17 00:00:00 2001
	From: =?UTF-8?q?Gerg=C5=91=20Tisza?= <tgr.huwiki@gmail.com>
	Date: Fri, 2 Sep 2022 21:51:50 -0700
	Subject: [PATCH] SECURITY: Prevent open redirect in
	Special:OAuth/rest_redirect

	Avoid an unrestricted redirect in Special:OAuth/rest_redirect by
	always prefixing with the server URL, instead of using wfExpandUrl()
	which leaves full URLs unchanged. Also simplify the logic by passing
	a single relative URL instead of path + query, and avoid PHP errors
	when the parameter is missing.

	Bug: T312820
	Change-Id: I789fb7384d89fbf42df22dc7b1953fb9087d95b1
	---
	src/Frontend/SpecialPages/SpecialMWOAuth.php \| 14 +++++++++-----
	src/Rest/Handler/Authorize.php \| 4 ++--
	2 files changed, 11 insertions(+), 7 deletions(-)

	diff --git a/src/Frontend/SpecialPages/SpecialMWOAuth.php b/src/Frontend/SpecialPages/SpecialMWOAuth.php
	index 33aaab9..7ad7c60 100644
	--- a/src/Frontend/SpecialPages/SpecialMWOAuth.php
	+++ b/src/Frontend/SpecialPages/SpecialMWOAuth.php
	@@ -262,13 +262,17 @@ class SpecialMWOAuth extends \UnlistedSpecialPage {

	case 'rest_redirect':
	$query = $this->getRequest()->getQueryValues();
	+ if ( !array_key_exists( 'rest_url', $query ) ) {
	+ throw new OAuthException( 'Invalid redirect' );
	+ }
	$restUrl = $query['rest_url'];
	- unset( $query['title'] );
	- unset( $query['rest_url'] );
	-
	- $target = wfExpandUrl( $restUrl );
	+ // make sure there's no way to change the domain
	+ if ( $restUrl[0] !== '/' ) {
	+ $restUrl = '/' . $restUrl;
	+ }
	+ $target = wfGetServerUrl( PROTO_CURRENT ) . $restUrl;

	- $output->redirect( wfAppendQuery( $target, $query ) );
	+ $output->redirect( $target );
	break;

	case '':
	diff --git a/src/Rest/Handler/Authorize.php b/src/Rest/Handler/Authorize.php
	index 504cb6d..304336f 100644
	--- a/src/Rest/Handler/Authorize.php
	+++ b/src/Rest/Handler/Authorize.php
	@@ -187,8 +187,8 @@ class Authorize extends AuthenticationHandler {
	return $this->getResponseFactory()->createTemporaryRedirect(
	SpecialPage::getTitleFor( 'Userlogin' )->getFullURL( [
	'returnto' => SpecialPage::getTitleFor( 'OAuth', 'rest_redirect' ),
	- 'returntoquery' => $this->getQueryParamsCgi( [
	- 'rest_url' => $this->getRequest()->getUri()->getPath()
	+ 'returntoquery' => wfArrayToCgi( [
	+ 'rest_url' => $this->getRequest()->getUri()->__toString(),
	] ),
	] )
	);
	--
	2.32.1 (Apple Git-133)

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 9741706
Default Alt Text: T312820.patch (2 KB)

T312820.patchmmartorana (manfredi martorana)Actions

T312820.patchView Options

File Metadata

Event Timeline

T312820.patch
mmartorana (manfredi martorana)
Actions

T312820.patch
View Options