Last modified: 2014-01-14 07:26:07 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 T56527, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 54527 - NULL pointer dereference in php-luasandbox
NULL pointer dereference in php-luasandbox
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Scribunto (Other open bugs)
unspecified
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: 59830
  Show dependency treegraph
 
Reported: 2013-09-24 20:14 UTC by Brad Jorsch
Modified: 2014-01-14 07:26 UTC (History)
9 users (show)

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


Attachments
Patch to php-luasandbox to handle the NULL (5.16 KB, patch)
2013-09-24 20:14 UTC, Brad Jorsch
Details
Patch to Scribunto to handle the same situation in LuaStandalone (3.90 KB, patch)
2013-09-24 20:17 UTC, Brad Jorsch
Details
Rebased patch for luasandbox (on top of bug 49705) (5.24 KB, patch)
2013-11-14 21:03 UTC, Brad Jorsch
Details
Changelog update for this and bug 49705 (967 bytes, patch)
2013-11-14 21:03 UTC, Brad Jorsch
Details
Rebased patch for Scribunto extension (3.91 KB, patch)
2013-11-14 21:04 UTC, Brad Jorsch
Details

Description Brad Jorsch 2013-09-24 20:14:03 UTC
Created attachment 13361 [details]
Patch to php-luasandbox to handle the NULL

As part of converting data structures from Lua to PHP, lua-phpsandbox will convert Lua tables to PHP arrays. To do this, it uses the lua_tolstring function to convert the table keys into strings for PHP.

Unfortunately, lua_tolstring will return NULL if it is asked to convert any data type besides number and string, and this possibility is not being checked before the resulting pointer is passed to zend_hash_update. zend_hash_update turns around and passes the pointer to zend_inline_hash_func, which dereferences the pointer and causes a crash.

This crash can be caused by any user simply by returning a table such as { [{}] = 1 } from a module function, or by passing such a table to PHP.

The attached patch fixes matters by checking for this NULL and throwing an error.
Comment 1 Brad Jorsch 2013-09-24 20:17:56 UTC
Created attachment 13362 [details]
Patch to Scribunto to handle the same situation in LuaStandalone

The LuaStandalone interpreter in Scribunto also doesn't handle these problematic keys correctly, although here there is no null pointer problem. It will, however, encode table and function keys using Lua's default tostring function which will expose the pointer addresses of tables and functions.

This patch probably isn't a security issue in itself, but anyone seeing it could easily think to try the same thing in LuaSandbox and discover the php-luasandbox bug. So I'm going to put this patch here too, at least for the moment.
Comment 2 Chris Steipp 2013-09-25 23:38:16 UTC
Can we get the package on the cluster updated for this issue this week, and we'll make this public next week with the next security release?
Comment 3 Tim Starling 2013-09-26 03:25:05 UTC
The patches look fine to me.
Comment 4 Chris Steipp 2013-11-14 19:30:40 UTC
This was assigned CVE-2013-4570
Comment 5 Brad Jorsch 2013-11-14 21:03:12 UTC
Created attachment 13796 [details]
Rebased patch for luasandbox (on top of bug 49705)
Comment 6 Brad Jorsch 2013-11-14 21:03:46 UTC
Created attachment 13797 [details]
Changelog update for this and bug 49705
Comment 7 Brad Jorsch 2013-11-14 21:04:11 UTC
Created attachment 13798 [details]
Rebased patch for Scribunto extension
Comment 8 Daniel Zahn 2013-12-03 19:38:04 UTC
built 1.8 on labs php-packaging instance. had to install these first this time, not sure why they were gone from the instance:  dpkg-checkbuilddeps: Unmet build dependencies: php5-dev liblua5.1-0-dev pkg-config
..then:

clone to /tmp..
cd /tmp/build/luasandboxorig/

git apply 0001-Fix-Lua-stack-overflow.patch
git apply 0002-Handle-invalid-keys-in-Lua-to-PHP-calls-for-LuaSandb.patch
git apply 0003-Update-changelog.patch

debuild -us -uc ..

Version: 1.8-1
Distribution: precise-wikimedia
Urgency: high
Maintainer: Tim Starling <tstarling@wikimedia.org>
Changed-By: Brad Jorsch <bjorsch@wikimedia.org>
..

 debdiff /home/dzahn/php-luasandbox_1.7-1_amd64.deb /tmp/build/php-luasandbox_1.8-1_amd64.deb 
File lists identical (after any substitutions)

Control files: lines which differ (wdiff format)
------------------------------------------------
Depends: libc6 (>= [-2.14),-] {+2.4),+} liblua5.1-0, php5-common
Installed-Size: [-126-] {+148+}
Version: [-1.7-1-] {+1.8-1+}


.. copied to test.wp - mw1017 -

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


Navigation
Links