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 WebMar 26 2015, 8:39 PM

Add Comment