Patch: API: Add "standard" header and hook for lacksSameOriginSecurity()
No One
Actions

Authored By

	Anomie
	Oct 26 2015, 5:51 PM

Size

5 KB

Referenced Files

None

Subscribers

None

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

	From da8839f444f92b915377e6257c6a623bb3df727f Mon Sep 17 00:00:00 2001
	From: Brad Jorsch <bjorsch@wikimedia.org>
	Date: Fri, 8 May 2015 10:20:30 -0400
	Subject: [PATCH] [SECURITY] 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
	---
	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 8d36603..334ab22 100644
	--- a/docs/hooks.txt
	+++ b/docs/hooks.txt
	@@ -2458,6 +2458,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 1465543..68d1df5 100644
	--- a/includes/api/ApiBase.php
	+++ b/includes/api/ApiBase.php
	@@ -422,7 +422,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 c641c95..9ef04c0 100644
	--- a/includes/api/ApiMain.php
	+++ b/includes/api/ApiMain.php
	@@ -145,6 +145,9 @@ class ApiMain extends ApiBase {
	private $mCacheControl = array();
	private $mParamsUsed = array();

	+ /** @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.
	*
	@@ -243,6 +246,36 @@ class ApiMain extends ApiBase {
	}

	/**
	+ * 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
	*/
	@@ -720,6 +753,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 aef4815..765d3bc 100644
	--- a/tests/phpunit/includes/api/ApiMainTest.php
	+++ b/tests/phpunit/includes/api/ApiMainTest.php
	@@ -252,4 +252,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.6.1

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 2882598
Default Alt Text: Patch: API: Add "standard" header and hook for lacksSameOriginSecurity() (5 KB)

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

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

File Metadata

Event Timeline

Patch: API: Add "standard" header and hook for lacksSameOriginSecurity()
No One
Actions

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