From 5b9942d04cce56a7c5a2e96a9705549b092d0c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Tisza?= 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)