From 7ac73b488f3cb996d8a54c086e0db35ced1893b3 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 a6ce3b2..d9a69db 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 189fd9f..5992175 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 eb23bd6..56a1d4d 100644 --- a/includes/api/ApiFormatBase.php +++ b/includes/api/ApiFormatBase.php @@ -65,6 +65,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 */ @@ -191,6 +211,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 228b47e..ebaeb2c 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'] ) || isset( $data['errors'] ) ) { diff --git a/includes/api/ApiHelp.php b/includes/api/ApiHelp.php index df9ca98..3434d92 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 e6f3fc4..3f037cc 100644 --- a/includes/api/ApiQuery.php +++ b/includes/api/ApiQuery.php @@ -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 ); + $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)