Last modified: 2014-01-14 07:27:29 UTC
Created attachment 12565 [details] Patch to fix the issue The underlying cause in bug 49649 is a buffer overflow in malloced memory in the luasandbox PHP extension. In Lua's C API, the caller is responsible for calling lua_checkstack to expand the Lua's stack as necessary before pushing onto it, and the various lua_push* functions apparently don't do any checking unless you compile with certain debug options activated. The lua_checkstack call isn't being done when processing arguments when calling from PHP to Lua or when returning values from PHP to Lua, so if the existing Lua stack isn't big enough (default is space for 20 values unless some other call already made Lua allocate a bigger stack) it will overflow into the next data structure on the heap. FYI, an easier reproduction in Scribunto than the one given in bug 49649 is the one liner mw.ustring.codepoint( string.rep( 'x', 1000 ), 1, -1); the limit on the 1000 depends on whether anything else has already increased the Lua stack size. An even simpler (but longer) one-liner is to replace the string.rep call with a literal string of sufficient length. It can also be reproduced from the command line, see the tests in the attached patch. Deployment process is apparently to update the changelog then have someone with appropriate access (Ops?) run the wmf-build script that's in the mediawiki/php/luasandbox repo. Tim usually does all that, of course, but he's on vacation this week.
Created attachment 12566 [details] Patch to update the changelog
How will we handle this one in Gerrit? Are we gonna submit it in public with the "fix buffer overflow" comment, but only right before deployment?
(In reply to comment #2) > How will we handle this one in Gerrit? Are we gonna submit it in public with > the "fix buffer overflow" comment, but only right before deployment? I know this was already answered on IRC, but for the record: We're handling it like any other security update, if possible: deploy the fix, then put it in Gerrit afterwards. The bug also gets reassigned to the appropriate non-Security product when it's closed. I don't know if there will be a security announcement too or what. Coordinate with Chris on the details and timing of the making-public bits, of course.
i added myself to labs project "packaging", and used instance "php-packaging" since it appears in the wmf-build script from the luasandbox repo. i applied the patch and build 1.7-1, then manually copied from the labs instance to brewster and imported with reprepro. since the package is "ensure latest" in puppet, it then got auto-updated on the cluster hosts. as of this writing i'ts now "ii php-luasandbox 1.7-1 " on all 217 mw* hosts.
test case as decribed in https://bugzilla.wikimedia.org/show_bug.cgi?id=49649 as far as i understand it should be this: http://en.wiktionary.org/w/index.php?title=User:Mutante/foo&action=history and http://en.wiktionary.org/wiki/User:Mutante/foo did _not_ become uneditable. So that would be good, please confirm.
(In reply to comment #5) > and http://en.wiktionary.org/wiki/User:Mutante/foo did _not_ become > uneditable. > So that would be good, please confirm. My tests work as well.
cool, claiming RESOLVED then.
This is not in gerrit yet, do we want to make it public now?
(In reply to comment #8) > This is not in gerrit yet, do we want to make it public now? I'd say let's ask Chris.
The only other issue is if there are external users we would want to notify. I'm assuming that's not the case here (although Brad, let us know if you think otherwise), so I think we can release-- i.e., put the patch in gerrit and send a note to wikitech-l in case other people want to patch.
It's certainly possible that others have downloaded and installed luasandbox, we mention it on the extension page and people have filed bugs against it specifically.
Created attachment 13158 [details] Rebased patch, and added one more check It would be nice to get this released sometime soon. In the mean time, I rebased the patch and added an additional luaL_checkstack() call.
Created attachment 13159 [details] And here's a version rebased on top of Gerrit change #63565 Also, since Gerrit change #63565 and this have conflicts, here's a version rebased on top of that change in case that gets merged first.
I'm planning to include this this with 1.21.2, currently scheduled for Aug 27th.
Created attachment 13226 [details] Changelog for 1.7 and 1.8
This was assigned CVE-2013-4571
Created attachment 13795 [details] Rebased patch
