From 9ec1ef7308acc0366e92f8e6af10ce3cb22b5065 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Fri, 8 May 2015 10:20:30 -0400
Subject: [PATCH] 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>
---
 docs/hooks.txt                             |  6 ++++
 includes/api/ApiBase.php                   |  8 ++++-
 includes/api/ApiMain.php                   | 35 ++++++++++++++++++++++
 tests/phpunit/includes/api/ApiMainTest.php | 27 +++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/docs/hooks.txt b/docs/hooks.txt
index f6527866a9..9d2b3c2be1 100644
--- 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 7d0ae32c6a..639f6be0b8 100644
--- 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 9c54eaced9..b4354b9233 100644
--- 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 06e7962fec..53554d7946 100644
--- 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' );
+	}
 }
-- 
2.20.1