HTML formatter for debug output from API allows HTML injection
Closed, ResolvedPublic

Assigned To
Anomie
Priority
Needs Triage
Author
Schnark
Blocks
Restricted Task
Subscribers
matmarex, Anomie, Schnark and 5 others
Projects
Reference
bz61362
Description

Steps to reproduce:

  1. Go to

https://en.wikipedia.org/w/api.php?action=parse&text=api.php?http://onmouseover=alert%28document.cookie%29//&title=Foo&prop=wikitext&format=jsonfm

  1. Hover over the first link.

-> A popup will be shown with your cookie.

The problem is in function formatHTML from ApiFormatBase.php:

The raw output contains

api.php?http://onmouseover=alert(document.cookie)//

In a first pass this is transformed into a link to api.php, i.e. into

<a href="api.php?http://onmouseover=alert(document.cookie)//">...</a>

In a second step the string starting with http:// are recognized as URLs and transformed into a link, too. But as it is inside an attribute this breaks the HTML structure:

<a href="api.php?<a href="http://onmouseover=alert(document.cookie)//">...</a>">...</a>

This is invalid HTML, but according to HTML5 the first a-tag gets an onmouseover-attribute with the value 'alert(document.cookie)//"'.


Version: unspecified
Severity: normal
URL: https://en.wikipedia.org/w/api.php?action=parse&text=api.php?http://onmouseover=alert%28document.cookie%29//&title=Foo&prop=wikitext&format=jsonfm

bzimport added a project: MediaWiki-API.Via ConduitNov 22 2014, 2:53 AM
bzimport set Reference to bz61362.
Schnark created this task.Via LegacyFeb 14 2014, 8:20 AM
Anomie added a comment.Via ConduitFeb 14 2014, 9:51 PM

Created attachment 14596
Fix

Ugh. This should fix it, by hiding the already-created links from the second pass of link finding.

Attached: 0001-API-Don-t-find-links-in-the-middle-of-api.php-links.patch

csteipp added a comment.Via ConduitFeb 20 2014, 7:40 PM

Patch looks good, and fixes the issue. I asked Aaron to take a look at it also. We should be able to deploy it soon.

Thanks for the report Michael.

aaron added a comment.Via ConduitFeb 20 2014, 9:28 PM

Would be nice if the patch used: <([0-9a-f]{40})> instead of <([0-9a-f]+)>. I think this is OK though.

csteipp added a comment.Via ConduitFeb 21 2014, 1:22 AM

I deployed the patch as it was today. Brad, it might be nice to make that update, just to make it a little more specific before we release this. Next release will probably be in a couple of weeks.

Bawolff added a comment.Via ConduitFeb 28 2014, 3:00 AM

For reference, this was commit Idf985e4e69c2f11778a8a90503914678441cb3fb (gerrit bot presumably can't edit hidden security bugs)

csteipp added a comment.Via ConduitMar 3 2014, 7:11 PM

This was assigned CVE-2014-2244

gerritbot added a comment.Via ConduitMar 7 2014, 7:30 PM

Change 115961 merged by jenkins-bot:
SECURITY: API: Don't find links in the middle of api.php links

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

csteipp added a project: Security.Via WebThu, Mar 26, 8:39 PM

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.