From: Brad Jorsch Date: Fri, 26 Feb 2016 22:46:07 +0000 (-0500) Subject: SECURITY: API: Avoid some silliness with browser-guessed filenames X-Git-Tag: 1.31.0-rc.0~1516 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=c75f0e95c9888489961548c72ef24786c43838aa;hp=25390162c755eb19077310fc04b8f3d19bf1dc23 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 --- diff --git a/api.php b/api.php index a6ce3b25e3..d9a69db37e 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 bc7747fe72..fd223e63dd 100644 --- a/includes/Feed.php +++ b/includes/Feed.php @@ -230,6 +230,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 06eaa19ca4..c5f2fcfaa7 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 @@ -192,6 +212,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 228b47ea65..ebaeb2ce25 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 318555a93b..ea4f724abf 100644 --- a/includes/api/ApiHelp.php +++ b/includes/api/ApiHelp.php @@ -62,6 +62,7 @@ class ApiHelp extends ApiBase { if ( $params['wrap'] ) { $data = [ 'mime' => 'text/html', + 'filename' => 'api-help.html', '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->addValue( null, 'filename', 'api-help.html', ApiResult::NO_SIZE_CHECK ); } } diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php index 44a46b8d16..31bcc7a704 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' ] );