SECURITY: API: Avoid some silliness with browser-guessed filenames
authorBrad Jorsch <bjorsch@wikimedia.org>
Fri, 26 Feb 2016 22:46:07 +0000 (17:46 -0500)
committerReedy <reedy@wikimedia.org>
Wed, 15 Nov 2017 00:58:44 +0000 (00:58 +0000)
If someone is both dumb enough to blindly save an API response and to
then execute the resulting file, this can be used to attack their
computer.

We can mitigate this by disallowing PATH_INFO in api.php URLs (because
we don't make any use of them anyway) and by setting a sensible filename
using a Content-Disposition header so the browser won't go guessing at
the filename based on what is in the URL.

Issue reported by: Abdullah Hussam

Bug: T128209
Change-Id: I8526f5cc506c551edb6138d68450b6acea065e93

api.php
includes/Feed.php
includes/api/ApiFormatBase.php
includes/api/ApiFormatRaw.php
includes/api/ApiHelp.php
includes/api/ApiQuery.php

diff --git a/api.php b/api.php
index a6ce3b2..d9a69db 100644 (file)
--- a/api.php
+++ b/api.php
@@ -44,6 +44,17 @@ if ( !$wgRequest->checkUrlExtension() ) {
        return;
 }
 
        return;
 }
 
+// Pathinfo can be used for stupid things. We don't support it for api.php at
+// all, so error out if it's present.
+if ( isset( $_SERVER['PATH_INFO'] ) && $_SERVER['PATH_INFO'] != '' ) {
+       $correctUrl = wfAppendQuery( wfScript( 'api' ), $wgRequest->getQueryValues() );
+       $correctUrl = wfExpandUrl( $correctUrl, PROTO_CANONICAL );
+       header( "Location: $correctUrl", true, 301 );
+       echo 'This endpoint does not support "path info", i.e. extra text between "api.php"'
+               . 'and the "?". Remove any such text and try again.';
+       die( 1 );
+}
+
 // Verify that the API has not been disabled
 if ( !$wgEnableAPI ) {
        header( $_SERVER['SERVER_PROTOCOL'] . ' 500 MediaWiki configuration Error', true, 500 );
 // Verify that the API has not been disabled
 if ( !$wgEnableAPI ) {
        header( $_SERVER['SERVER_PROTOCOL'] . ' 500 MediaWiki configuration Error', true, 500 );
index bc7747f..fd223e6 100644 (file)
@@ -230,6 +230,12 @@ abstract class ChannelFeed extends FeedItem {
                $wgOut->disable();
                $mimetype = $this->contentType();
                header( "Content-type: $mimetype; charset=UTF-8" );
                $wgOut->disable();
                $mimetype = $this->contentType();
                header( "Content-type: $mimetype; charset=UTF-8" );
+
+               // Set a sane filename
+               $exts = MimeMagic::singleton()->getExtensionsForType( $mimetype );
+               $ext = $exts ? strtok( $exts, ' ' ) : 'xml';
+               header( "Content-Disposition: inline; filename=\"feed.{$ext}\"" );
+
                if ( $wgVaryOnXFP ) {
                        $wgOut->addVaryHeader( 'X-Forwarded-Proto' );
                }
                if ( $wgVaryOnXFP ) {
                        $wgOut->addVaryHeader( 'X-Forwarded-Proto' );
                }
index 06eaa19..c5f2fcf 100644 (file)
@@ -64,6 +64,26 @@ abstract class ApiFormatBase extends ApiBase {
         */
        abstract public function getMimeType();
 
         */
        abstract public function getMimeType();
 
+       /**
+        * Return a filename for this module's output.
+        * @note If $this->getIsWrappedHtml() || $this->getIsHtml(), you'll very
+        *  likely want to fall back to this class's version.
+        * @since 1.27
+        * @return string Generally this should be "api-result.$ext", and must be
+        *  encoded for inclusion in a Content-Disposition header's filename parameter.
+        */
+       public function getFilename() {
+               if ( $this->getIsWrappedHtml() ) {
+                       return 'api-result-wrapped.json';
+               } elseif ( $this->getIsHtml() ) {
+                       return 'api-result.html';
+               } else {
+                       $exts = MimeMagic::singleton()->getExtensionsForType( $this->getMimeType() );
+                       $ext = $exts ? strtok( $exts, ' ' ) : strtolower( $this->mFormat );
+                       return "api-result.$ext";
+               }
+       }
+
        /**
         * Get the internal format name
         * @return string
        /**
         * Get the internal format name
         * @return string
@@ -192,6 +212,13 @@ abstract class ApiFormatBase extends ApiBase {
                if ( $apiFrameOptions ) {
                        $this->getMain()->getRequest()->response()->header( "X-Frame-Options: $apiFrameOptions" );
                }
                if ( $apiFrameOptions ) {
                        $this->getMain()->getRequest()->response()->header( "X-Frame-Options: $apiFrameOptions" );
                }
+
+               // Set a Content-Disposition header so something downloading an API
+               // response uses a halfway-sensible filename (T128209).
+               $filename = $this->getFilename();
+               $this->getMain()->getRequest()->response()->header(
+                       "Content-Disposition: inline; filename=\"{$filename}\""
+               );
        }
 
        /**
        }
 
        /**
index 228b47e..ebaeb2c 100644 (file)
@@ -60,6 +60,17 @@ class ApiFormatRaw extends ApiFormatBase {
                return $data['mime'];
        }
 
                return $data['mime'];
        }
 
+       public function getFilename() {
+               $data = $this->getResult()->getResultData();
+               if ( isset( $data['error'] ) ) {
+                       return $this->errorFallback->getFilename();
+               } elseif ( !isset( $data['filename'] ) || $this->getIsWrappedHtml() || $this->getIsHtml() ) {
+                       return parent::getFilename();
+               } else {
+                       return $data['filename'];
+               }
+       }
+
        public function initPrinter( $unused = false ) {
                $data = $this->getResult()->getResultData();
                if ( isset( $data['error'] ) || isset( $data['errors'] ) ) {
        public function initPrinter( $unused = false ) {
                $data = $this->getResult()->getResultData();
                if ( isset( $data['error'] ) || isset( $data['errors'] ) ) {
index 318555a..ea4f724 100644 (file)
@@ -62,6 +62,7 @@ class ApiHelp extends ApiBase {
                if ( $params['wrap'] ) {
                        $data = [
                                'mime' => 'text/html',
                if ( $params['wrap'] ) {
                        $data = [
                                'mime' => 'text/html',
+                               'filename' => 'api-help.html',
                                'help' => $html,
                        ];
                        ApiResult::setSubelementsList( $data, 'help' );
                                'help' => $html,
                        ];
                        ApiResult::setSubelementsList( $data, 'help' );
@@ -70,6 +71,7 @@ class ApiHelp extends ApiBase {
                        $result->reset();
                        $result->addValue( null, 'text', $html, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( null, 'mime', 'text/html', ApiResult::NO_SIZE_CHECK );
                        $result->reset();
                        $result->addValue( null, 'text', $html, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( null, 'mime', 'text/html', ApiResult::NO_SIZE_CHECK );
+                       $result->addValue( null, 'filename', 'api-help.html', ApiResult::NO_SIZE_CHECK );
                }
        }
 
                }
        }
 
index 44a46b8..31bcc7a 100644 (file)
@@ -459,6 +459,7 @@ class ApiQuery extends ApiBase {
                        // Raw formatter will handle this
                        $result->addValue( null, 'text', $sink, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( null, 'mime', 'text/xml', ApiResult::NO_SIZE_CHECK );
                        // Raw formatter will handle this
                        $result->addValue( null, 'text', $sink, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( null, 'mime', 'text/xml', ApiResult::NO_SIZE_CHECK );
+                       $result->addValue( null, 'filename', 'export.xml', ApiResult::NO_SIZE_CHECK );
                } else {
                        $result->addValue( 'query', 'export', $sink, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( 'query', ApiResult::META_BC_SUBELEMENTS, [ 'export' ] );
                } else {
                        $result->addValue( 'query', 'export', $sink, ApiResult::NO_SIZE_CHECK );
                        $result->addValue( 'query', ApiResult::META_BC_SUBELEMENTS, [ 'export' ] );