jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/391411 )

Change subject: SECURITY: API: Avoid some silliness with browser-guessed 
filenames
......................................................................


SECURITY: API: Avoid some silliness with browser-guessed filenames

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
---
M api.php
M includes/Feed.php
M includes/api/ApiFormatBase.php
M includes/api/ApiFormatRaw.php
M includes/api/ApiHelp.php
M includes/api/ApiQuery.php
6 files changed, 58 insertions(+), 0 deletions(-)

Approvals:
  jenkins-bot: Verified
  Ejegg: Looks good to me, approved



diff --git a/api.php b/api.php
index 6e75fb7..7362137 100644
--- a/api.php
+++ b/api.php
@@ -44,6 +44,17 @@
        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 );
diff --git a/includes/Feed.php b/includes/Feed.php
index 8bfe1c7..882a449 100644
--- a/includes/Feed.php
+++ b/includes/Feed.php
@@ -232,6 +232,12 @@
                $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' );
                }
diff --git a/includes/api/ApiFormatBase.php b/includes/api/ApiFormatBase.php
index c826bba..af6fcc8 100644
--- a/includes/api/ApiFormatBase.php
+++ b/includes/api/ApiFormatBase.php
@@ -64,6 +64,26 @@
        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
         */
@@ -173,6 +193,13 @@
                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}\""
+               );
        }
 
        /**
diff --git a/includes/api/ApiFormatRaw.php b/includes/api/ApiFormatRaw.php
index 9da040c..d73e3dc 100644
--- a/includes/api/ApiFormatRaw.php
+++ b/includes/api/ApiFormatRaw.php
@@ -60,6 +60,17 @@
                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'] ) ) {
diff --git a/includes/api/ApiHelp.php b/includes/api/ApiHelp.php
index 0f0fbdc..6aad4a1 100644
--- a/includes/api/ApiHelp.php
+++ b/includes/api/ApiHelp.php
@@ -61,6 +61,7 @@
                if ( $params['wrap'] ) {
                        $data = [
                                'mime' => 'text/html',
+                               'filename' => 'api-help.html',
                                'help' => $html,
                        ];
                        ApiResult::setSubelementsList( $data, 'help' );
@@ -69,6 +70,7 @@
                        $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 );
                }
        }
 
diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php
index ed4d373..57fac71 100644
--- a/includes/api/ApiQuery.php
+++ b/includes/api/ApiQuery.php
@@ -462,6 +462,7 @@
                        // Raw formatter will handle this
                        $result->addValue( null, 'text', $exportxml, 
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', $exportxml, 
ApiResult::NO_SIZE_CHECK );
                        $result->addValue( 'query', 
ApiResult::META_BC_SUBELEMENTS, [ 'export' ] );

-- 
To view, visit https://gerrit.wikimedia.org/r/391411
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I8526f5cc506c551edb6138d68450b6acea065e93
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: fundraising/REL1_27
Gerrit-Owner: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to