API: Add "standard" header and hook for lacksSameOriginSecurity()

The header is intended for use with XMLHttpRequest when the request
might be part of an XSS. The hook is for extensions that might need to
add additional checks of some sort.

Bug: T98313
Change-Id: I0e5f2d3b29a79a12461dc33c90c812a56810f536

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>

diff --git a/docs/hooks.txt b/docs/hooks.txt
index f652786..9d2b3c2 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -2494,6 +2494,12 @@ $context: (IContextSource) The RequestContext the skin is being created for.
 &$skin: A variable reference you may set a Skin instance or string key on to
   override the skin that will be used for the context.
 
+'RequestHasSameOriginSecurity': Called to determine if the request is somehow
+flagged to lack same-origin security. Return false to indicate the lack. Note
+if the "somehow" involves HTTP headers, you'll probably need to make sure
+the header is varied on.
+WebRequest $request: The request.
+
 'ResetPasswordExpiration': Allow extensions to set a default password expiration
 $user: The user having their password expiration reset
 &$newExpire: The new expiration date

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 7d0ae32..639f6be 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -503,7 +503,13 @@ abstract class ApiBase extends ContextSource {
         * @return bool
         */
        public function lacksSameOriginSecurity() {
-               return $this->getMain()->getRequest()->getVal( 'callback' ) !== null;
+               // Main module has this method overridden
+               // Safety - avoid infinite loop:
+               if ( $this->isMain() ) {
+                       ApiBase::dieDebug( __METHOD__, 'base method was called on main module.' );
+               }
+
+               return $this->getMain()->lacksSameOriginSecurity();
        }
 
        /**

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 9c54eac..b4354b9 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -148,6 +148,9 @@ class ApiMain extends ApiBase {
        private $mCacheControl = [];
        private $mParamsUsed = [];
 
+       /** @var bool|null Cached return value from self::lacksSameOriginSecurity() */
+       private $lacksSameOriginSecurity = null;
+
        /**
         * Constructs an instance of ApiMain that utilizes the module and format specified by $request.
         *
@@ -245,6 +248,36 @@ class ApiMain extends ApiBase {
                return $this->mResult;
        }
 
+       /**
+        * Get the security flag for the current request
+        * @return bool
+        */
+       public function lacksSameOriginSecurity() {
+               if ( $this->lacksSameOriginSecurity !== null ) {
+                       return $this->lacksSameOriginSecurity;
+               }
+
+               $request = $this->getRequest();
+
+               // JSONP mode
+               if ( $request->getVal( 'callback' ) !== null ) {
+                       $this->lacksSameOriginSecurity = true;
+                       return true;
+               }
+
+               // Header to be used from XMLHTTPRequest when the request might
+               // otherwise be used for XSS.
+               if ( $request->getHeader( 'Treat-as-Untrusted' ) !== false ) {
+                       $this->lacksSameOriginSecurity = true;
+                       return true;
+               }
+
+               // Allow extensions to override.
+               $this->lacksSameOriginSecurity = !Hooks::run( 'RequestHasSameOriginSecurity', array( $request ) );
+               return $this->lacksSameOriginSecurity;
+       }
+
+
        /**
         * Get the ApiErrorFormatter object associated with current request
         * @return ApiErrorFormatter
@@ -730,6 +763,8 @@ class ApiMain extends ApiBase {
                $response = $this->getRequest()->response();
                $out = $this->getOutput();
 
+               $out->addVaryHeader( 'Treat-as-Untrusted' );
+
                $config = $this->getConfig();
 
                if ( $config->get( 'VaryOnXFP' ) ) {

diff --git a/tests/phpunit/includes/api/ApiMainTest.php b/tests/phpunit/includes/api/ApiMainTest.php
index 06e7962..53554d7 100644 (file)
--- a/tests/phpunit/includes/api/ApiMainTest.php
+++ b/tests/phpunit/includes/api/ApiMainTest.php
@@ -253,4 +253,31 @@ class ApiMainTest extends ApiTestCase {
                ];
        }
 
+       /**
+        * @covers ApiMain::lacksSameOriginSecurity
+        */
+       public function testLacksSameOriginSecurity() {
+               // Basic test
+               $main = new ApiMain( new FauxRequest( array( 'action' => 'query', 'meta' => 'siteinfo' ) ) );
+               $this->assertFalse( $main->lacksSameOriginSecurity(), 'Basic test, should have security' );
+
+               // JSONp
+               $main = new ApiMain(
+                       new FauxRequest( array( 'action' => 'query', 'format' => 'xml', 'callback' => 'foo'  ) )
+               );
+               $this->assertTrue( $main->lacksSameOriginSecurity(), 'JSONp, should lack security' );
+
+               // Header
+               $request = new FauxRequest( array( 'action' => 'query', 'meta' => 'siteinfo' ) );
+               $request->setHeader( 'TrEaT-As-UnTrUsTeD', '' ); // With falsey value!
+               $main = new ApiMain( $request );
+               $this->assertTrue( $main->lacksSameOriginSecurity(), 'Header supplied, should lack security' );
+
+               // Hook
+               $this->mergeMwGlobalArrayValue( 'wgHooks', array(
+                       'RequestHasSameOriginSecurity' => array( function () { return false; } )
+               ) );
+               $main = new ApiMain( new FauxRequest( array( 'action' => 'query', 'meta' => 'siteinfo' ) ) );
+               $this->assertTrue( $main->lacksSameOriginSecurity(), 'Hook, should lack security' );
+       }
 }