From 3e176d9e0462844144c031ec73d8aa6bd1c7b94b Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 26 Feb 2016 17:46:07 -0500 Subject: [PATCH] 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 --- api.php | 11 +++++++++++ includes/Feed.php | 6 ++++++ includes/api/ApiFormatBase.php | 27 +++++++++++++++++++++++++++ includes/api/ApiFormatRaw.php | 11 +++++++++++ includes/api/ApiHelp.php | 2 ++ includes/api/ApiQuery.php | 1 + 6 files changed, 58 insertions(+) diff --git a/api.php b/api.php index 6e75fb7..7362137 100644 --- a/api.php +++ b/api.php @@ -44,6 +44,17 @@ if ( !$wgRequest->checkUrlExtension() ) { 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 @@ abstract class ChannelFeed extends FeedItem { $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 4c406a7..9c8b819 100644 --- a/includes/api/ApiFormatBase.php +++ b/includes/api/ApiFormatBase.php @@ -64,6 +64,26 @@ abstract class ApiFormatBase extends ApiBase { 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 @@ abstract class ApiFormatBase extends ApiBase { 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 @@ class ApiFormatRaw extends ApiFormatBase { 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 02efd7b..49e5785 100644 --- a/includes/api/ApiHelp.php +++ b/includes/api/ApiHelp.php @@ -61,6 +61,7 @@ class ApiHelp extends ApiBase { if ( $params['wrap'] ) { $data = [ 'mime' => 'text/html', + 'filename' => 'api-help.html', 'help' => $html, ]; ApiResult::setSubelementsList( $data, 'help' ); @@ -69,6 +70,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->addValue( null, 'filename', 'api-help.html', ApiResult::NO_SIZE_CHECK ); } } diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php index 16bd725..16a2343 100644 --- a/includes/api/ApiQuery.php +++ b/includes/api/ApiQuery.php @@ -461,6 +461,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 ); + $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' ] ); -- 1.9.5 (Apple Git-50.3)