Page MenuHomePhabricator

NULL pointer dereference in php-luasandbox
Closed, ResolvedPublic

Description

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.


Version: unspecified
Severity: normal

attachment diff1 ignored as obsolete

Details

Reference
bz54527

Related Objects

StatusSubtypeAssignedTask
ResolvedNone

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:15 AM
bzimport added a project: Scribunto.
bzimport set Reference to bz54527.
bzimport added a subscriber: Unknown Object (MLST).

Created attachment 13362
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.

attachment diff2 ignored as obsolete

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?

Created attachment 13796
Rebased patch for luasandbox (on top of bug 49705)

Attached:

Created attachment 13797
Changelog update for this and bug 49705

Attached:

Created attachment 13798
Rebased patch for Scribunto extension

Attached:

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 -