Last modified: 2013-09-04 22:09:28 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T48084, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 46084 - Script injection in Scribunto profiling report
Script injection in Scribunto profiling report
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
unspecified
All All
: Unprioritized critical (vote)
: 1.20.x release
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-13 20:30 UTC by Brad Jorsch
Modified: 2013-09-04 22:09 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Patch per discussion with csteipp on IRC (1.08 KB, patch)
2013-03-13 20:47 UTC, Brad Jorsch
Details

Description Brad Jorsch 2013-03-13 20:30:55 UTC
If over one second of CPU time is used in Lua, the Lua functions using the most CPU time will be reported in the "NewPP limit report" HTML comment. And Lua functions can be named pretty much anything. Which means that {{#invoke:Evil|hello}} with the following as Module:Evil will be bad:

 local p = {}
 
 function p.hello()
     p['--><script>alert("uh oh")</script>']()
     for i = 0, 1e8 do end
 end
 
 p['--><script>alert("uh oh")</script>'] = function ()
     for i = 0, 1e9 do end
 end
 
 return p

(adjust the 1e8 and 1e9 to suit the speed of whatever wiki you're testing it on). You can test this live, right now, by going to https://en.wikipedia.org/w/index.php?title=Module:Bananas&action=edit, pasting the above in, and entering "Template:Lua hello world" in the TemplateSandbox field.

It should be simple enough to fix; probably the best thing would be to escape $limitReport at includes/parser/Parser.php line 504. The question is what exactly needs to be escaped to make the "NewPP limit report" comment safe, and hopefully still leave it readable? Would something as simple as

 str_replace( "--", "‐‐", $engine->getLimitReport() )

(that's U+2010) work?
Comment 1 Brad Jorsch 2013-03-13 20:40:01 UTC
I should proofread better. That last bit should be

 str_replace( "--", "‐‐", $limitReport )
Comment 2 Brad Jorsch 2013-03-13 20:47:25 UTC
Created attachment 11927 [details]
Patch per discussion with csteipp on IRC
Comment 3 Gerrit Notification Bot 2013-04-15 20:48:23 UTC
Related URL: https://gerrit.wikimedia.org/r/59197 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)
Comment 4 Gerrit Notification Bot 2013-04-15 22:32:36 UTC
Related URL: https://gerrit.wikimedia.org/r/59339 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)
Comment 5 Gerrit Notification Bot 2013-04-15 23:02:53 UTC
Related URL: https://gerrit.wikimedia.org/r/59345 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)
Comment 6 Gerrit Notification Bot 2013-04-15 23:26:54 UTC
Related URL: https://gerrit.wikimedia.org/r/59355 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)
Comment 7 Gerrit Notification Bot 2013-04-16 06:09:07 UTC
Related URL: https://gerrit.wikimedia.org/r/59375 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)
Comment 8 MZMcBride 2013-04-17 02:07:49 UTC
Injection into debug HTML comments like this is a fairly old trick. :-/  I really wish we would just remove this HTML comment from the output instead of further hacking up the parser in order to sanitize it.
Comment 9 Chris Steipp 2013-09-04 22:09:28 UTC
Redhat assigned CVE-2013-1951 for this issue (sorry for the late update)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links