From ce4ec2bdc39c9bc242f066a58203677bdb4c1e44 Mon Sep 17 00:00:00 2001 From: csteipp Date: Tue, 10 Feb 2015 17:32:48 -0800 Subject: [PATCH] SECURITY: Check limits before parsing XML To prevent various DoS attacks in our XML parsing, check that PHP has sane time and memory limits set before doing the parsing. Users can set $wgAllowUnsafeLimits = true if they want to use these features without setting any limits. Bug: T85848 Change-Id: Ib95edc9c4ad2247f2b58016e8128bcbf9109af6d --- includes/DefaultSettings.php | 7 +++++++ includes/GlobalFunctions.php | 14 ++++++++++++++ includes/Setup.php | 6 ++++++ includes/libs/XmlReaderTypeCheck.php | 10 ++++++++++ includes/libs/XmlTypeCheck.php | 8 ++++++++ includes/media/SVGMetadataExtractor.php | 7 +++++++ includes/media/XMP.php | 7 +++++++ 7 files changed, 59 insertions(+) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index f9dc1ed..c38ee7d 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -7404,6 +7404,13 @@ $wgPageLanguageUseDB = false; $wgUseLinkNamespaceDBFields = true; /** + * Allow processing user input that could potentially consume significant resources, without + * ensuring that sane limits are set by PHP. + * @var bool + */ +$wgAllowUnsafeLimits = false; + +/** * For really cool vim folding this needs to be at the end: * vim: foldmarker=@{,@} foldmethod=marker * @} diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 2025e17..c03cd18 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -4198,3 +4198,17 @@ function wfIsConfiguredProxy( $ip ) { wfDeprecated( __METHOD__, '1.24' ); return IP::isConfiguredProxy( $ip ); } + +/** + * Check resource limits, unless wgAllowUnsafeLimits is set. + * @return bool + */ +function wfCheckLimits() { + global $wgAllowUnsafeLimits; + if ( !$wgAllowUnsafeLimits && + ( ini_get( 'memory_limit' ) === -1 || ini_get( 'max_execution_time' ) === 0 ) + ) { + return false; + } + return true; +} diff --git a/includes/Setup.php b/includes/Setup.php index ed0aabe..9dd1daf 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -675,6 +675,12 @@ foreach ( $wgExtensionFunctions as $func ) { wfDebug( "Fully initialised\n" ); $wgFullyInitialised = true; +if ( !$wgCommandLineMode && wfIsHHVM() && !wfCheckLimits() ) { + wfWarn( 'Running without memory_limit or max_execution_time defined might allow ' . + 'some DoS attacks to succeed.' ); +} + + Profiler::instance()->scopedProfileOut( $ps_extensions ); Profiler::instance()->scopedProfileOut( $ps_setup ); diff --git a/includes/libs/XmlReaderTypeCheck.php b/includes/libs/XmlReaderTypeCheck.php index e859bac..f84e73c 100644 --- a/includes/libs/XmlReaderTypeCheck.php +++ b/includes/libs/XmlReaderTypeCheck.php @@ -77,10 +77,20 @@ class XmlReaderTypeCheck { * filename (default, true) or if it is a string (false) * @param array $options list of additional parsing options: * processing_instruction_handler: Callback for xml_set_processing_instruction_handler + * @throws MWException */ function __construct( $input, $filterCallback = null, $isFile = true, $options = array() ) { $this->filterCallback = $filterCallback ?: function() { return false; } ; $this->parserOptions = array_merge( $this->parserOptions, $options ); + + // Since we intentionally expand entities, make sure sane resource limits + // are setup - T85848 + if ( !wfCheckLimits() ) { + throw new MWException( "Cannot safely expand entities, needed to properly" . + " check the XML. Set PHP's memory_limit and max_execution_time" . + " to use this feature." ); + } + $this->validateFromInput( $input, $isFile ); } diff --git a/includes/libs/XmlTypeCheck.php b/includes/libs/XmlTypeCheck.php index 0d6c3a6..25cfaca 100644 --- a/includes/libs/XmlTypeCheck.php +++ b/includes/libs/XmlTypeCheck.php @@ -74,11 +74,18 @@ class XmlTypeCheck { * filename (default, true) or if it is a string (false) * @param array $options list of additional parsing options: * processing_instruction_handler: Callback for xml_set_processing_instruction_handler + * @throws MWException */ function __construct( $input, $filterCallback = null, $isFile = true, $options = array() ) { $this->filterCallback = $filterCallback; $this->parserOptions = array_merge( $this->parserOptions, $options ); + // Since hhvm expands entities, make sure sane resource limits are set - T85848 + if ( wfIsHHVM() && !wfCheckLimits() ) { + throw new MWException( "Please set PHP's memory_limit and max_execution_time " . + "to safely use this function." ); + } + if ( $isFile ) { $this->validateFromFile( $input ); } else { @@ -141,6 +148,7 @@ class XmlTypeCheck { array( $this, 'processingInstructionHandler' ) ); } + return $parser; } diff --git a/includes/media/SVGMetadataExtractor.php b/includes/media/SVGMetadataExtractor.php index 2037c33..06ab648 100644 --- a/includes/media/SVGMetadataExtractor.php +++ b/includes/media/SVGMetadataExtractor.php @@ -74,6 +74,13 @@ class SVGReader { throw new MWException( "Error getting filesize of SVG." ); } + // Since we intentionally expand entities, make sure sane resource limits + // are setup - T85848 + if ( !wfCheckLimits() ) { + throw new MWException( "Please set PHP's memory_limit and max_execution_time " . + "to safely use this function." ); + } + if ( $size > $wgSVGMetadataCutoff ) { $this->debug( "SVG is $size bytes, which is bigger than $wgSVGMetadataCutoff. Truncating." ); $contents = file_get_contents( $source, false, null, -1, $wgSVGMetadataCutoff ); diff --git a/includes/media/XMP.php b/includes/media/XMP.php index 0d341aa..d774d2c 100644 --- a/includes/media/XMP.php +++ b/includes/media/XMP.php @@ -112,6 +112,7 @@ class XMPReader { * Constructor. * * Primary job is to initialize the XMLParser + * @throws MWException */ function __construct() { @@ -120,6 +121,12 @@ class XMPReader { throw new MWException( 'XMP support requires XML Parser' ); } + // Since hhvm expands entities, make sure sane resource limits are set - T85848 + if ( wfIsHHVM() && !wfCheckLimits() ) { + throw new MWException( "Please set PHP's memory_limit and max_execution_time " . + "to safely use this function." ); + } + $this->items = XMPInfo::getItems(); $this->resetXMLParser(); -- 1.8.4.5