summaryrefslogtreecommitdiff
path: root/includes
diff options
context:
space:
mode:
authorPierre Schmitz <pierre@archlinux.de>2014-09-25 13:59:03 +0200
committerPierre Schmitz <pierre@archlinux.de>2014-09-25 13:59:03 +0200
commit364b55264cf4daafa7a5d353f9be71864307a0d6 (patch)
tree0cde72fb14997624fe7feb44c73a881406777d77 /includes
parent8ca31c4bfc61c8d99eda1cee307ff4de22897708 (diff)
Update to MediaWiki 1.22.11
Diffstat (limited to 'includes')
-rw-r--r--includes/DefaultSettings.php2
-rw-r--r--includes/Sanitizer.php51
-rw-r--r--includes/XmlTypeCheck.php57
-rw-r--r--includes/upload/UploadBase.php100
4 files changed, 180 insertions, 30 deletions
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index b2c12001..1ec2ea35 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -63,7 +63,7 @@ $wgConf = new SiteConfiguration;
* MediaWiki version number
* @since 1.2
*/
-$wgVersion = '1.22.10';
+$wgVersion = '1.22.11';
/**
* Name of the site. It must be changed in LocalSettings.php
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index 9e9ac38b..3ca66443 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -819,24 +819,16 @@ class Sanitizer {
}
/**
- * Pick apart some CSS and check it for forbidden or unsafe structures.
- * Returns a sanitized string. This sanitized string will have
- * character references and escape sequences decoded and comments
- * stripped (unless it is itself one valid comment, in which case the value
- * will be passed through). If the input is just too evil, only a comment
- * complaining about evilness will be returned.
- *
- * Currently URL references, 'expression', 'tps' are forbidden.
- *
- * NOTE: Despite the fact that character references are decoded, the
- * returned string may contain character references given certain
- * clever input strings. These character references must
- * be escaped before the return value is embedded in HTML.
- *
- * @param $value String
- * @return String
+ * Normalize CSS into a format we can easily search for hostile input
+ * - decode character references
+ * - decode escape sequences
+ * - convert characters that IE6 interprets into ascii
+ * - remove comments, unless the entire value is one single comment
+ * @param string $value the css string
+ * @return string normalized css
*/
- static function checkCss( $value ) {
+ public static function normalizeCss( $value ) {
+
// Decode character references like &#123;
$value = Sanitizer::decodeCharReferences( $value );
@@ -922,6 +914,31 @@ class Sanitizer {
$value
);
+ return $value;
+ }
+
+
+ /**
+ * Pick apart some CSS and check it for forbidden or unsafe structures.
+ * Returns a sanitized string. This sanitized string will have
+ * character references and escape sequences decoded and comments
+ * stripped (unless it is itself one valid comment, in which case the value
+ * will be passed through). If the input is just too evil, only a comment
+ * complaining about evilness will be returned.
+ *
+ * Currently URL references, 'expression', 'tps' are forbidden.
+ *
+ * NOTE: Despite the fact that character references are decoded, the
+ * returned string may contain character references given certain
+ * clever input strings. These character references must
+ * be escaped before the return value is embedded in HTML.
+ *
+ * @param string $value
+ * @return string
+ */
+ static function checkCss( $value ) {
+ $value = self::normalizeCss( $value );
+
// Reject problematic keywords and control characters
if ( preg_match( '/[\000-\010\013\016-\037\177]/', $value ) ) {
return '/* invalid control char */';
diff --git a/includes/XmlTypeCheck.php b/includes/XmlTypeCheck.php
index eb98a4ad..aca857e9 100644
--- a/includes/XmlTypeCheck.php
+++ b/includes/XmlTypeCheck.php
@@ -40,6 +40,23 @@ class XmlTypeCheck {
public $rootElement = '';
/**
+ * A stack of strings containing the data of each xml element as it's processed. Append
+ * data to the top string of the stack, then pop off the string and process it when the
+ * element is closed.
+ */
+ protected $elementData = array();
+
+ /**
+ * A stack of element names and attributes, as we process them.
+ */
+ protected $elementDataContext = array();
+
+ /**
+ * Current depth of the data stack.
+ */
+ protected $stackDepth = 0;
+
+ /**
* Additional parsing options
*/
private $parserOptions = array(
@@ -51,7 +68,7 @@ class XmlTypeCheck {
* @param callable $filterCallback (optional)
* Function to call to do additional custom validity checks from the
* SAX element handler event. This gives you access to the element
- * namespace, name, and attributes, but not to text contents.
+ * namespace, name, attributes, and text contents.
* Filter should return 'true' to toggle on $this->filterMatch
* @param boolean $isFile (optional) indicates if the first parameter is a
* filename (default, true) or if it is a string (false)
@@ -179,7 +196,12 @@ class XmlTypeCheck {
$this->rootElement = $name;
if ( is_callable( $this->filterCallback ) ) {
- xml_set_element_handler( $parser, array( $this, 'elementOpen' ), false );
+ xml_set_element_handler(
+ $parser,
+ array( $this, 'elementOpen' ),
+ array( $this, 'elementClose' )
+ );
+ xml_set_character_data_handler( $parser, array( $this, 'elementData' ) );
$this->elementOpen( $parser, $name, $attribs );
} else {
// We only need the first open element
@@ -193,7 +215,26 @@ class XmlTypeCheck {
* @param $attribs
*/
private function elementOpen( $parser, $name, $attribs ) {
- if ( call_user_func( $this->filterCallback, $name, $attribs ) ) {
+ $this->elementDataContext[] = array( $name, $attribs );
+ $this->elementData[] = '';
+ $this->stackDepth++;
+ }
+
+ /**
+ * @param $parser
+ * @param $name
+ */
+ private function elementClose( $parser, $name ) {
+ list( $name, $attribs ) = array_pop( $this->elementDataContext );
+ $data = array_pop( $this->elementData );
+ $this->stackDepth--;
+
+ if ( call_user_func(
+ $this->filterCallback,
+ $name,
+ $attribs,
+ $data
+ ) ) {
// Filter hit!
$this->filterMatch = true;
}
@@ -201,6 +242,16 @@ class XmlTypeCheck {
/**
* @param $parser
+ * @param $data
+ */
+ private function elementData( $parser, $data ) {
+ // xml_set_character_data_handler breaks the data on & characters, so
+ // we collect any data here, and we'll run the callback in elementClose
+ $this->elementData[ $this->stackDepth - 1 ] .= trim( $data );
+ }
+
+ /**
+ * @param $parser
* @param $target
* @param $data
*/
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 40b3b19a..26cc5427 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1201,7 +1201,8 @@ abstract class UploadBase {
* @param $attribs array
* @return bool
*/
- public function checkSvgScriptCallback( $element, $attribs ) {
+ public function checkSvgScriptCallback( $element, $attribs, $data = null ) {
+
list( $namespace, $strippedElement ) = $this->splitXmlNamespace( $element );
static $validNamespaces = array(
@@ -1274,6 +1275,14 @@ abstract class UploadBase {
return true;
}
+ # Check <style> css
+ if ( $strippedElement == 'style'
+ && self::checkCssFragment( Sanitizer::normalizeCss( $data ) )
+ ) {
+ wfDebug( __METHOD__ . ": hostile css in style element.\n" );
+ return true;
+ }
+
foreach ( $attribs as $attrib => $value ) {
$stripped = $this->stripXmlNamespace( $attrib );
$value = strtolower( $value );
@@ -1310,6 +1319,18 @@ abstract class UploadBase {
return true;
}
+ # Change href with animate from (http://html5sec.org/#137). This doesn't seem
+ # possible without embedding the svg, but filter here in case.
+ if ( $stripped == 'from'
+ && $strippedElement === 'animate'
+ && !preg_match( '!^https?://!im', $value )
+ ) {
+ wfDebug( __METHOD__ . ": Found animate that might be changing href using from "
+ . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
+
+ return true;
+ }
+
# use set/animate to add event-handler attribute to parent
if ( ( $strippedElement == 'set' || $strippedElement == 'animate' ) && $stripped == 'attributename' && substr( $value, 0, 2 ) == 'on' ) {
wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with \"<$strippedElement $stripped='$value'...\" in uploaded file.\n" );
@@ -1335,14 +1356,23 @@ abstract class UploadBase {
}
# use CSS styles to bring in remote code
- # catch url("http:..., url('http:..., url(http:..., but not url("#..., url('#..., url(#....
- if ( $stripped == 'style' && preg_match_all( '!((?:font|clip-path|fill|filter|marker|marker-end|marker-mid|marker-start|mask|stroke)\s*:\s*url\s*\(\s*["\']?\s*[^#]+.*?\))!sim', $value, $matches ) ) {
- foreach ( $matches[1] as $match ) {
- if ( !preg_match( '!(?:font|clip-path|fill|filter|marker|marker-end|marker-mid|marker-start|mask|stroke)\s*:\s*url\s*\(\s*(#|\'#|"#)!sim', $match ) ) {
- wfDebug( __METHOD__ . ": Found svg setting a style with remote url '$attrib'='$value' in uploaded file.\n" );
- return true;
- }
- }
+ if ( $stripped == 'style'
+ && self::checkCssFragment( Sanitizer::normalizeCss( $value ) )
+ ) {
+ wfDebug( __METHOD__ . ": Found svg setting a style with "
+ . "remote url '$attrib'='$value' in uploaded file.\n" );
+ return true;
+ }
+
+ # Several attributes can include css, css character escaping isn't allowed
+ $cssAttrs = array( 'font', 'clip-path', 'fill', 'filter', 'marker',
+ 'marker-end', 'marker-mid', 'marker-start', 'mask', 'stroke' );
+ if ( in_array( $stripped, $cssAttrs )
+ && self::checkCssFragment( $value )
+ ) {
+ wfDebug( __METHOD__ . ": Found svg setting a style with "
+ . "remote url '$attrib'='$value' in uploaded file.\n" );
+ return true;
}
# image filters can pull in url, which could be svg that executes scripts
@@ -1357,6 +1387,58 @@ abstract class UploadBase {
}
/**
+ * Check a block of CSS or CSS fragment for anything that looks like
+ * it is bringing in remote code.
+ * @param string $value a string of CSS
+ * @param bool $propOnly only check css properties (start regex with :)
+ * @return bool true if the CSS contains an illegal string, false if otherwise
+ */
+ private static function checkCssFragment( $value ) {
+
+ # Forbid external stylesheets, for both reliability and to protect viewer's privacy
+ if ( strpos( $value, '@import' ) !== false ) {
+ return true;
+ }
+
+ # We allow @font-face to embed fonts with data: urls, so we snip the string
+ # 'url' out so this case won't match when we check for urls below
+ $pattern = '!(@font-face\s*{[^}]*src:)url(\("data:;base64,)!im';
+ $value = preg_replace( $pattern, '$1$2', $value );
+
+ # Check for remote and executable CSS. Unlike in Sanitizer::checkCss, the CSS
+ # properties filter and accelerator don't seem to be useful for xss in SVG files.
+ # Expression and -o-link don't seem to work either, but filtering them here in case.
+ # Additionally, we catch remote urls like url("http:..., url('http:..., url(http:...,
+ # but not local ones such as url("#..., url('#..., url(#....
+ if ( preg_match( '!expression
+ | -o-link\s*:
+ | -o-link-source\s*:
+ | -o-replace\s*:!imx', $value ) ) {
+ return true;
+ }
+
+ if ( preg_match_all(
+ "!(\s*(url|image|image-set)\s*\(\s*[\"']?\s*[^#]+.*?\))!sim",
+ $value,
+ $matches
+ ) !== 0
+ ) {
+ # TODO: redo this in one regex. Until then, url("#whatever") matches the first
+ foreach ( $matches[1] as $match ) {
+ if ( !preg_match( "!\s*(url|image|image-set)\s*\(\s*(#|'#|\"#)!im", $match ) ) {
+ return true;
+ }
+ }
+ }
+
+ if ( preg_match( '/[\000-\010\013\016-\037\177]/', $value ) ) {
+ return true;
+ }
+
+ return false;
+ }
+
+ /**
* Divide the element name passed by the xml parser to the callback into URI and prifix.
* @param $name string
* @return array containing the namespace URI and prefix