summaryrefslogtreecommitdiff
path: root/includes/upload
diff options
context:
space:
mode:
authorLuke Shumaker <lukeshu@sbcglobal.net>2015-02-25 23:38:25 -0500
committerLuke Shumaker <lukeshu@sbcglobal.net>2015-02-25 23:38:25 -0500
commitb0e5922cdadff2b394100dc8977bc2d526c04595 (patch)
treef1c19b1aaf0988cdef72f978b9f16c5d631d3727 /includes/upload
parentad2b9dc3e492af9d550532817f34f865a97a8f63 (diff)
parentb88ab0086858470dd1f644e64cb4e4f62bb2be9b (diff)
Merge commit 'b88ab'
Diffstat (limited to 'includes/upload')
-rw-r--r--includes/upload/UploadBase.php104
1 files changed, 93 insertions, 11 deletions
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 40b3b19a..8268f8e3 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 );
@@ -1294,8 +1303,8 @@ abstract class UploadBase {
wfDebug( __METHOD__ . ": Found href attribute <$strippedElement "
. "'$attrib'='$value' in uploaded file.\n" );
- return true;
- }
+ return true;
+ }
}
# href with embedded svg as target
@@ -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