Page MenuHomePhabricator

T128209-REL1_29.patch

Authored By
Bawolff
Nov 9 2017, 9:08 PM
Size
6 KB
Referenced Files
None
Subscribers
None

T128209-REL1_29.patch

From 7ac73b488f3cb996d8a54c086e0db35ced1893b3 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
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)

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5077770
Default Alt Text
T128209-REL1_29.patch (6 KB)

Event Timeline