From 3eacf0349fe7ff6f2c831831501037c92a2185c6 Mon Sep 17 00:00:00 2001
From: Derk-Jan Hartman <hartman.wiki@gmail.com>
Date: Thu, 4 Dec 2014 15:39:50 +0100
Subject: [PATCH] Only return CORS headers in the response as required

- Split out responses of preflight and actual CORS requests
- If the request is not CORS valid, don't set the CORS response headers

Note that invalid CORS requests should not actually throw error
responses, the client should simply not handle the response because the
response does not have the right headers (it's a client side policy
error not an http error). We do throw a 403 for a mismatch with the
queryparam, but since that is 'outside' of the spec, that might be
appropriate.

Bug: T76701
Change-Id: Ib296c68babe5c0b380268ee7793b3d6d35b9c3e3
---
 includes/api/ApiMain.php | 73 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index a5287b6a10..ba22e39457 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -487,6 +487,8 @@ class ApiMain extends ApiBase {
 	 * If the parameter and the header do match, the header is checked against $wgCrossSiteAJAXdomains
 	 * and $wgCrossSiteAJAXdomainExceptions, and if the origin qualifies, the appropriate CORS
 	 * headers are set.
+	 * http://www.w3.org/TR/cors/#resource-requests
+	 * http://www.w3.org/TR/cors/#resource-preflight-requests
 	 *
 	 * @return bool False if the caller should abort (403 case), true otherwise (all other cases)
 	 */
@@ -499,12 +501,14 @@ class ApiMain extends ApiBase {
 
 		$request = $this->getRequest();
 		$response = $request->response();
+
 		// Origin: header is a space-separated list of origins, check all of them
 		$originHeader = $request->getHeader( 'Origin' );
 		if ( $originHeader === false ) {
 			$origins = array();
 		} else {
-			$origins = explode( ' ', $originHeader );
+			$originHeader = trim( $originHeader );
+			$origins = preg_split( '/\s+/', $originHeader );
 		}
 
 		if ( !in_array( $originParam, $origins ) ) {
@@ -519,19 +523,43 @@ class ApiMain extends ApiBase {
 		}
 
 		$config = $this->getConfig();
-		$matchOrigin = self::matchOrigin(
+		$matchOrigin = count( $origins ) === 1 && self::matchOrigin(
 			$originParam,
 			$config->get( 'CrossSiteAJAXdomains' ),
 			$config->get( 'CrossSiteAJAXdomainExceptions' )
 		);
 
 		if ( $matchOrigin ) {
-			$response->header( "Access-Control-Allow-Origin: $originParam" );
+			$requestedMethod = $request->getHeader( 'Access-Control-Request-Method' );
+			$preflight = $request->getMethod() === 'OPTIONS' && $requestedMethod !== false;
+			if ( $preflight ) {
+				// This is a CORS preflight request
+				if ( $requestedMethod !== 'POST' && $requestedMethod !== 'GET' ) {
+					// If method is not a case-sensitive match, do not set any additional headers and terminate.
+					return true;
+				}
+				// We allow the actual request to send the following headers
+				$requestedHeaders = $request->getHeader( 'Access-Control-Request-Headers' );
+				if ( $requestedHeaders !== false ) {
+					if ( !self::matchRequestedHeaders( $requestedHeaders ) ) {
+						return true;
+					}
+					$response->header( 'Access-Control-Allow-Headers: ' . $requestedHeaders );
+				}
+
+				// We only allow the actual request to be GET or POST
+				$response->header( 'Access-Control-Allow-Methods: POST, GET' );
+			}
+
+			$response->header( "Access-Control-Allow-Origin: $originHeader" );
 			$response->header( 'Access-Control-Allow-Credentials: true' );
-			$response->header( 'Access-Control-Allow-Headers: Api-User-Agent' );
-			$this->getOutput()->addVaryHeader( 'Origin' );
+
+			if ( !$preflight ) {
+				$response->header( 'Access-Control-Expose-Headers: MediaWiki-API-Error, Retry-After, X-Database-Lag' );
+			}
 		}
 
+		$this->getOutput()->addVaryHeader( 'Origin' );
 		return true;
 	}
 
@@ -560,6 +588,41 @@ class ApiMain extends ApiBase {
 		return false;
 	}
 
+	/**
+	 * Attempt to validate the value of Access-Control-Request-Headers against a list
+	 * of headers that we allow the follow up request to send.
+	 *
+	 * @param string $requestedHeaders Comma seperated list of HTTP headers
+	 * @return bool True if all requested headers are in the list of allowed headers
+	 */
+	protected static function matchRequestedHeaders( $requestedHeaders ) {
+		if ( trim( $requestedHeaders ) === '' ) {
+			return true;
+		}
+		$requestedHeaders = explode( ',', $requestedHeaders );
+		$allowedAuthorHeaders = array_flip( array(
+			/* simple headers (see spec) */
+			'accept',
+			'accept-language',
+			'content-language',
+			'content-type',
+			/* non-authorable headers in XHR, which are however requested by some UAs */
+			'accept-encoding',
+			'dnt',
+			'origin',
+			/* MediaWiki whitelist */
+			'api-user-agent',
+		) );
+		foreach ( $requestedHeaders as $rHeader ) {
+			$rHeader = strtolower( trim( $rHeader ) );
+			if ( !isset( $allowedAuthorHeaders[$rHeader] ) ) {
+				wfDebugLog( 'api', 'CORS preflight failed on requested header: ' . $rHeader );
+				return false;
+			}
+		}
+		return true;
+	}
+
 	/**
 	 * Helper function to convert wildcard string into a regex
 	 * '*' => '.*?'
-- 
2.20.1