Page MenuHomePhabricator

Stored XSS in SVG via embedded SVG
Closed, ResolvedPublic

Description

FINDING ID: iSEC-WMF1214-11

DESCRIPTION: When uploading an SVG file, it is possible to bypass the validation filters and upload an
SVG file that executes JavaScript when rendered. The SVG validation is a blacklist-based approach and
contains protections against numerous techniques for embedding JavaScript in SVG files; however, the
protections against nested SVG files are inadequate due to a missing MIME type blacklist. Specifically,
data: URIs with the image/svg or text/xml formats are blacklisted, but the application/xml MIME
type is sufficient to create a nested SVG file with JavaScript code.

# /includes/upload/UploadBase.php
# href with embedded svg as target
if ($stripped == 'href' && preg_match( '!data:[^,]*image/svg[^,]*,!sim', $value)){
wfDebug( __METHOD__ . ": Found href to embedded svg "
. "\"<$strippedElement '$attrib'='$value '...\" in uploaded file.\n" );
return true;}
# href with embedded (text/xml) svg as target
if ( $stripped == 'href' && preg_match( '!data:[^,]*text/xml[^,]*,!sim', $value)){
wfDebug( __METHOD__ . ": Found href to embedded svg "
. "\"<$strippedElement '$attrib'='$value '...\" in uploaded file.\n" );
return true;}

Listing 1: Blacklist filtering for data: URIs with specific MIME types.

The following example SVG file contains a nested SVG file embedded within a data: URI. The nested
SVG file contains executable JavaScript.

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<use xlink:href="data:application/xml;base64 ,
PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5r
PSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIj4KPGRlZnM+CjxjaXJjbGUgaWQ9InRlc3QiIHI9I
jUwIiBjeD0iMTAwIiBjeT0iMTAwIiBzdHlsZT0iZmlsbDogI0YwMCI+CjxzZXQgYXR0cmlidXRlTm
FtZT0iZmlsbCIgYXR0cmlidXRlVHlwZT0iQ1NTIiBvbmJlZ2luPSdhbGVydChkb2N1bWVudC5jb29r
aWUpJwpvbmVuZD0nYWxlcnQoIm9uZW5kIiknIHRvPSIjMDBGIiBiZWdpbj0iMXMiIGR1cj0iNXMiIC
8+CjwvY2lyY2xlPgo8L2RlZnM+Cjx1c2UgeGxpbms6aHJlZj0iI3Rlc3QiLz4KPC9zdmc+#test"/>
</svg>

Listing 2: Specially-crafted SVG file with a nested SVG file containing executable JavaScript code.

During testing, this exploit was only confirmed against the Firefox browser. Testing indicates that other
browsers do not support nested SVG files


Patch:

(based on top of patch for T85349, )

  • 1.24:
  • 1.23:
  • 1.19:

Affected Versions: (needed)
Type: xss
CVE: CVE-2015-2931

Event Timeline

csteipp created this task.Jan 5 2015, 8:46 PM
csteipp raised the priority of this task from to Medium.
csteipp updated the task description. (Show Details)
csteipp changed the visibility from "Public (No Login Required)" to "acl*security (Project)".
csteipp changed the edit policy from "All Users" to "acl*security (Project)".
csteipp changed Security from None to Software security bug.
csteipp added a subscriber: csteipp.
Restricted Application changed the visibility from "acl*security (Project)" to "Custom Policy". · View Herald TranscriptJan 5 2015, 8:46 PM
Restricted Application changed the edit policy from "acl*security (Project)" to "Custom Policy". · View Herald Transcript
csteipp claimed this task.EditedJan 13 2015, 1:05 AM

This patch is based on top of @Parent5446's patch for T85349 (

)

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 13 2015, 1:05 AM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 13 2015, 1:07 AM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 13 2015, 1:09 AM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald Transcript
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
Anomie added a subscriber: Anomie.Jan 13 2015, 6:52 PM

This does seem to address the vulnerability, so +1 if you need it.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 13 2015, 6:52 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
csteipp closed this task as Resolved.Jan 20 2015, 11:11 PM

This is deployed, and all other work for it is done. This will be included in the 1.24.2 release.

csteipp reopened this task as Open.EditedJan 28 2015, 12:26 AM

Reopening this issue based on the report from cure53, who found a similar vector.

Cure53 agreed a whitelist of mime types would be more sufficient than attempting to blacklist anything that could be interpreted as an svg.

I parsed through my 27k svg images, and this is the complete list of href data url mime types:
data:;base64,
data:image/gif;base64,
data:image/jpeg;base64,
data:image/jpg;base64,
data:image/png;base64,

There were 28 with no mime type specified. They were a mix of b64 encoded image types (png and jpg). My inclination is to whitelist the image/(gif|jpeg|jpeg|png) mime types and reject images which don't specify a mime type, although that would reject more incoming images than I like.

New patch based on top of the other UploadBase.php patches. I tested against 22,000 commons images with master and this applied, and only the images with "data:;base64," were rejected, as expected.

This will need some community engagement to clarify, but I think it's an easy enough rule to add onto http://commons.wikimedia.org/wiki/Help:SVG.

Depending on how far we want to go with supporting attribute-value pairs in the regex, that ";[\w;]+" bit could be as complicated as this:

(?>;[a-zA-Z0-9!#$&\'*+.^_`{|}~-]+=(?>[a-zA-Z0-9!#$&\'*+.^_`{|}~-]+|"(?>[\0-\x0c\x0e-\x21\x23-\x5b\x5d-\x7f]+|\\[\0-\x7f])*"))*(?:;base64)?

(I've already included the necessary backslash-before-single-quote in there, BTW).

Updated with @Anomie's regex updates. In testing, it's about 10% slower than (;[\w;]+)*, but we then know it's well formed.

Oops, that actually needs to be \\\\[\0-\x7f] in there. Stupid backslashes.

I wonder whether preg_match( '!^data:!i', $value ) should be replaced by strncasecmp( 'data:', $value, 5 ) === 0. Regex against an anchored constant just seems wrong.

Updated to fix those things Brad pointed out. Thanks!

Looks good. +1.

csteipp closed this task as Resolved.Feb 17 2015, 11:56 PM

deployed

bd808 moved this task from Done to Archive on the MediaWiki-Core-Team board.Feb 23 2015, 11:58 PM

combined patches to simplify backporting

csteipp updated the task description. (Show Details)Mar 18 2015, 1:10 AM

- whitespace differences in UploadBase
- typo fix, whitespace differences, and no unit tests for UploadBase

csteipp updated the task description. (Show Details)Mar 24 2015, 8:09 PM
csteipp changed the visibility from "Custom Policy" to "Custom Policy".Mar 31 2015, 12:39 PM
csteipp changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 31 2015, 9:16 PM
csteipp changed the edit policy from "Custom Policy" to "All Users".
csteipp changed Security from Software security bug to None.

Change 201012 had a related patch set uploaded (by CSteipp):
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201012

Change 201024 had a related patch set uploaded (by CSteipp):
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201024

Change 201034 had a related patch set uploaded (by CSteipp):
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201034

Change 201012 merged by jenkins-bot:
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201012

Change 201024 merged by jenkins-bot:
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201024

Change 201034 merged by jenkins-bot:
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201034

Change 201218 had a related patch set uploaded (by CSteipp):
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201218

Change 201218 merged by jenkins-bot:
SECURITY: Don't allow embedded application/xml in SVG's

https://gerrit.wikimedia.org/r/201218

Asasigned CVE-2015-2931

Use CVE-2015-2931 for this issue involving an incomplete list of

disallowed MIME types for data: URIs (the application/xml type wasn't
in this list). In other words, CVE-2015-2931 does not refer more
generally to the desirability of the "MIME types are now whitelisted"
decision.

csteipp updated the task description. (Show Details)Apr 8 2015, 1:58 AM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:42 PM