Last modified: 2013-11-15 11:08:01 UTC
Lua code is expected to be sandboxed so that one #invoke session can never impact another invoke session, and so that certain sensitive functions cannot be run from Mediawiki Modules. I've discovered a set of behaviors that allows one to violate the sandbox. A working (though weak) version of this exploit can be seen using: http://test2.wikipedia.org/wiki/Module:MessageTest1 http://test2.wikipedia.org/wiki/Module:MessageTest2 Executed at: http://test2.wikipedia.org/wiki/Module_talk:MessageTest2 This example shows how the user can pass messages from one invoke session to another. When MessageTest1 is loaded via require (or mw.loadData), for some reason its table ("message") appears to be stored in the global environment. Subsequently, one can use an indirect call _G.message to access functions loaded from MessageTest1. Those functions then appear to run in the global space and be able to violate the sandboxing rules. In this example by storing a variable in the global space to be retrieved later. However, I don't think there are any limits to what those functions are allowed to do. A related problem is that data loaded via require can also overload functions and values in the global space even without using the _G. construction. For example, is a required Module contains the code "mw.clone = {};" then once required the global function mw.clone will be overwritten. This causes all future invoke calls to fail since mw.clone is required to run at the start of each Lua session. Require calls shouldn't be overwriting functions in the global space, and the sandboxing needs to be fixed so that code loaded via require can't later be used to access the global space.
A sandbox violation is when you can call, say, os.execute(). Module isolation is a different thing from sandboxing and is not a security issue. The ability to modify the base environment is a bit concerning, but I don't think that's a security issue either, since you can't even generate PHP warnings after mw.setupInterface() removes the mw_interface global. You do get access to setfenv() and getfenv() but in the standalone engine, they are already wrappers, and in the sandbox engine, they are harmless. So, changing component. This appears to be due to I92a47d31. A package module is created in the base environment, and package.lua has: -- -- avoid overwriting the package table if it's already there -- package = package or {} ... package.loaders = package.loaders or { loader_preload } So the loadPackage() closure from the base environment is retained in the cloned environment, and so loaded chunks have their environment set to the base environment before they are called. Replacing "package = package or {}" with "package = {}" appears to fix the problem, and all tests still pass after that is done. Assigning to Brad.
(In reply to comment #1) > So the loadPackage() closure from the base environment is retained in the > cloned environment, and so loaded chunks have their environment set to the > base environment before they are called. I found the same thing. > Replacing "package = package or {}" with "package = {}" appears to fix the > problem, and all tests still pass after that is done. Other fixes include only doing this to package.loaders, or clearing env.package.loaders in mw.lua's makePackageModule. I'd prefer either of these, so package.loaded continues to contain entries for the various Scribunto libraries. There's a similar hole in mw.loadData, where it uses the outer sandbox's require and so the loaded module was again getting the outer sandbox's environment.
Related URL: https://gerrit.wikimedia.org/r/59576 (Gerrit Change I48d8dd4784c9a890e3abb6389f96f38e1420dbbb)
https://gerrit.wikimedia.org/r/59576 (Gerrit Change I48d8dd4784c9a890e3abb6389f96f38e1420dbbb) | change APPROVED and MERGED [by Tim Starling]
Change merged. Note the fix should be deployed on WMF wikis with 1.22wmf3; see https://www.mediawiki.org/wiki/MediaWiki_1.22/Roadmap for the schedule.
Change 95402 had a related patch set uploaded by MarkAHershberger: (bug 47300) Fix sandboxing with require https://gerrit.wikimedia.org/r/95402
Change 95402 abandoned by MarkAHershberger: (bug 47300) Fix sandboxing with require https://gerrit.wikimedia.org/r/95402
No open patches to review here (backport patches got abandoned), hence resetting status to RESOLVED FIXED. Backport_to_Stable flag might be set to "-" by hexmode.
