r114624 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r114623‎ | r114624 | r114625 >
Date:03:30, 30 March 2012
Author:tstarling
Status:deferred
Tags:
Comment:
* Implemented pcall() and xpcall() per Victor's request. Added tests.
* Refactored timeout error generation, since it's a bit more complicated now
* Used "static inline" instead of plain "inline" functions in headers per the recommendation at http://www.greenend.org.uk/rjk/tech/inline.html
Modified paths:
  • /trunk/php/luasandbox/data_conversion.c (modified) (history)
  • /trunk/php/luasandbox/library.c (modified) (history)
  • /trunk/php/luasandbox/luasandbox.c (modified) (history)
  • /trunk/php/luasandbox/php_luasandbox.h (modified) (history)
  • /trunk/php/luasandbox/tests/pcall.phpt (added) (history)
  • /trunk/php/luasandbox/tests/xpcall.phpt (added) (history)
  • /trunk/php/luasandbox/timer.c (modified) (history)

Diff [purge]

Index: trunk/php/luasandbox/php_luasandbox.h
——@@ -22,7 +22,6 @@
2323 /* luasandbox.c */
2424
2525 extern zend_module_entry luasandbox_module_entry;
26 -extern char luasandbox_timeout_message[];
2726
2827 #define phpext_luasandbox_ptr &luasandbox_module_entry
2928
——@@ -104,12 +103,12 @@
105104 * unsafe to call longjmp() to return control to PHP. If the flag is not
106105 * correctly set, memory may be corrupted and security compromised.
107106 */
108 -inline void luasandbox_enter_php(lua_State * L, php_luasandbox_obj * intern)
 107+static inline void luasandbox_enter_php(lua_State * L, php_luasandbox_obj * intern)
109108 {
110109 intern->in_php ++;
111110 if (intern->timed_out) {
112111 intern->in_php --;
113 - luaL_error(L, luasandbox_timeout_message);
 112+ luasandbox_timer_timeout_error(L);
114113 }
115114 }
116115 /* }}} */
——@@ -119,7 +118,7 @@
120119 * This function must be called after luasandbox_enter_php, before the callback
121120 * from Lua returns.
122121 */
123 -inline void luasandbox_leave_php(lua_State * L, php_luasandbox_obj * intern)
 122+static inline void luasandbox_leave_php(lua_State * L, php_luasandbox_obj * intern)
124123 {
125124 intern->in_php --;
126125 }
——@@ -138,6 +137,10 @@
139138 void luasandbox_push_zval_userdata(lua_State * L, zval * z);
140139 void luasandbox_lua_to_zval(zval * z, lua_State * L, int index,
141140 zval * sandbox_zval, HashTable * recursionGuard TSRMLS_DC);
 141+void luasandbox_wrap_fatal(lua_State * L);
 142+int luasandbox_is_fatal(lua_State * L, int index);
 143+char * luasandbox_error_to_string(lua_State * L, int index);
142144
 145+
143146 #endif /* PHP_LUASANDBOX_H */
144147
Index: trunk/php/luasandbox/data_conversion.c
——@@ -19,6 +19,13 @@
2020 extern zend_class_entry *luasandboxfunction_ce;
2121 extern zend_class_entry *luasandboxplaceholder_ce;
2222
 23+/**
 24+ * An int, the address of which is used as a fatal error marker. The value is
 25+ * not used.
 26+ */
 27+int luasandbox_fatal_error_marker = 0;
 28+
 29+
2330 /** {{{ luasandbox_data_conversion_init
2431 *
2532 * Set up a lua_State so that this module can work with it.
——@@ -339,3 +346,79 @@
340347 }
341348 /* }}} */
342349
 350+/** {{{ luasandbox_wrap_fatal
 351+ *
 352+ * Pop a value off the top of the stack, and push a fatal error wrapper
 353+ * containing the value.
 354+ */
 355+void luasandbox_wrap_fatal(lua_State * L)
 356+{
 357+ // Create the table and put the marker in it as element 1
 358+ lua_createtable(L, 0, 2);
 359+ lua_pushlightuserdata(L, &luasandbox_fatal_error_marker);
 360+ lua_rawseti(L, -2, 1);
 361+
 362+ // Swap the table with the input value, so that the value is on the top,
 363+ // then put the value in the table as element 2
 364+ lua_insert(L, -2);
 365+ lua_rawseti(L, -2, 2);
 366+}
 367+/* }}} */
 368+
 369+/** {{{ luasandbox_is_fatal
 370+ *
 371+ * Check if the value at the given stack index is a fatal error wrapper
 372+ * created by luasandbox_wrap_fatal(). Return 1 if it is, 0 otherwise.
 373+ */
 374+int luasandbox_is_fatal(lua_State * L, int index)
 375+{
 376+ void * ud;
 377+ int haveIndex2 = 0;
 378+
 379+ if (!lua_istable(L, index)) {
 380+ return 0;
 381+ }
 382+
 383+ lua_rawgeti(L, index, 1);
 384+ ud = lua_touserdata(L, -1);
 385+ lua_pop(L, 1);
 386+ if (ud != &luasandbox_fatal_error_marker) {
 387+ return 0;
 388+ }
 389+
 390+ lua_rawgeti(L, index, 2);
 391+ haveIndex2 = !lua_isnil(L, -1);
 392+ lua_pop(L, 1);
 393+ return haveIndex2;
 394+}
 395+/* }}} */
 396+
 397+/** {{{
 398+ *
 399+ * If the value at the given stack index is a fatal error wrapper, convert
 400+ * the error object it wraps to a string. If the value is anything else,
 401+ * convert it directly to a string. If the error object is not convertible
 402+ * to a string, return "unknown error".
 403+ *
 404+ * This calls lua_tolstring() and will corrupt the value on the stack as
 405+ * described in that function's documentation. The string is valid until the
 406+ * Lua value is destroyed.
 407+ */
 408+char * luasandbox_error_to_string(lua_State * L, int index)
 409+{
 410+ char * s;
 411+ if (luasandbox_is_fatal(L, index)) {
 412+ lua_rawgeti(L, index, 2);
 413+ s = lua_tostring(L, -1);
 414+ lua_pop(L, 1);
 415+ } else {
 416+ s = lua_tostring(L, index);
 417+ }
 418+ if (!s) {
 419+ return "unknown error";
 420+ } else {
 421+ return s;
 422+ }
 423+}
 424+/* }}} */
 425+
Index: trunk/php/luasandbox/luasandbox.c
——@@ -44,8 +44,6 @@
4545 static int luasandbox_call_php(lua_State * L);
4646 static int luasandbox_dump_writer(lua_State * L, const void * p, size_t sz, void * ud);
4747
48 -char luasandbox_timeout_message[] = "The maximum execution time for this script was exceeded";
49 -
5048 zend_class_entry *luasandbox_ce;
5149 zend_class_entry *luasandboxerror_ce;
5250 zend_class_entry *luasandboxemergencytimeout_ce;
——@@ -393,7 +391,7 @@
394392 {
395393 php_error_docref(NULL TSRMLS_CC, E_ERROR,
396394 "PANIC: unprotected error in call to Lua API (%s)",
397 - lua_tostring(L, -1));
 395+ luasandbox_error_to_string(L, -1));
398396 return 0;
399397 }
400398 /* }}} */
——@@ -535,7 +533,7 @@
536534 */
537535 static void luasandbox_handle_error(lua_State * L, int status)
538536 {
539 - const char * errorMsg = lua_tostring(L, -1);
 537+ const char * errorMsg = luasandbox_error_to_string(L, -1);
540538 lua_pop(L, 1);
541539 if (!EG(exception)) {
542540 zend_throw_exception(luasandboxerror_ce, (char*)errorMsg, status);
——@@ -1117,7 +1115,9 @@
11181116
11191117 // If an exception occurred, convert it to a Lua error (just to unwind the stack)
11201118 if (EG(exception)) {
1121 - luaL_error(L, "[exception]");
 1119+ lua_pushstring(L, "[exception]");
 1120+ luasandbox_wrap_fatal(L);
 1121+ lua_error(L);
11221122 }
11231123 return num_results;
11241124 }
Index: trunk/php/luasandbox/timer.c
——@@ -20,6 +20,7 @@
2121 struct timespec * normal_timeout,
2222 struct timespec * emergency_timeout) {}
2323 void luasandbox_timer_stop(luasandbox_timer_set * lts) {}
 24+void luasandbox_timer_timeout_error(lua_State *L) {}
2425
2526 #else
2627
——@@ -29,6 +30,8 @@
3031 static void luasandbox_timer_timeout_hook(lua_State *L, lua_Debug *ar);
3132 static void luasandbox_timer_settime(luasandbox_timer * lt, struct timespec * ts);
3233
 34+char luasandbox_timeout_message[] = "The maximum execution time for this script was exceeded";
 35+
3336 void luasandbox_timer_install_handler(struct sigaction * oldact)
3437 {
3538 struct sigaction newact;
——@@ -46,6 +49,7 @@
4750 static void luasandbox_timer_handle(int signo, siginfo_t * info, void * context)
4851 {
4952 luasandbox_timer_callback_data * data;
 53+ lua_State * L;
5054
5155 if (signo != LUASANDBOX_SIGNAL
5256 || info->si_code != SI_TIMER
——@@ -56,6 +60,7 @@
5761
5862 data = (luasandbox_timer_callback_data*)info->si_value.sival_ptr;
5963 data->sandbox->timed_out = 1;
 64+ L = data->sandbox->state;
6065 if (data->emergency) {
6166 sigset_t set;
6267 sigemptyset(&set);
——@@ -68,11 +73,13 @@
6974 "was exceeded and a PHP callback failed to return");
7075 } else {
7176 // The Lua state is dirty now and can't be used again.
72 - luaL_error(data->sandbox->state, "emergency timeout");
 77+ lua_pushstring(L, "emergency timeout");
 78+ luasandbox_wrap_fatal(L);
 79+ lua_error(L);
7380 }
7481 } else {
7582 // Set a hook which will terminate the script execution in a graceful way
76 - lua_sethook(data->sandbox->state, luasandbox_timer_timeout_hook,
 83+ lua_sethook(L, luasandbox_timer_timeout_hook,
7784 LUA_MASKCALL | LUA_MASKRET | LUA_MASKLINE, 1);
7885 }
7986 }
——@@ -82,9 +89,16 @@
8390 // Avoid infinite recursion
8491 lua_sethook(L, luasandbox_timer_timeout_hook, 0, 0);
8592 // Do a longjmp to report the timeout error
86 - luaL_error(L, luasandbox_timeout_message);
 93+ luasandbox_timer_timeout_error(L);
8794 }
8895
 96+void luasandbox_timer_timeout_error(lua_State *L)
 97+{
 98+ lua_pushstring(L, luasandbox_timeout_message);
 99+ luasandbox_wrap_fatal(L);
 100+ lua_error(L);
 101+}
 102+
89103 void luasandbox_timer_start(luasandbox_timer_set * lts,
90104 php_luasandbox_obj * sandbox,
91105 struct timespec * normal_timeout,
Index: trunk/php/luasandbox/tests/xpcall.phpt
——@@ -0,0 +1,102 @@
 2+--TEST--
 3+xpcall() basic behaviour
 4+--FILE--
 5+<?php
 6+
 7+$lua = <<<LUA
 8+ function xpcall_test(f, err)
 9+ local status, msg
 10+ status, msg = xpcall(f, err)
 11+ if not status then
 12+ return msg
 13+ else
 14+ return "success"
 15+ end
 16+ end
 17+LUA;
 18+
 19+$xperr = 'return "xp: " .. msg';
 20+
 21+$tests = array(
 22+ 'Normal' => array(
 23+ 'return 1',
 24+ $xperr
 25+ ),
 26+ 'User error' => array(
 27+ 'error("runtime error")',
 28+ $xperr
 29+ ),
 30+ 'Error in error handler' => array(
 31+ 'error("original error")',
 32+ 'error("error in handler")'
 33+ ),
 34+ 'Unconvertible error in error handler' => array(
 35+ 'error("original error")',
 36+ 'error({})'
 37+ ),
 38+ 'Numeric error in error handler' => array(
 39+ 'error("original error")',
 40+ 'error(2)',
 41+ ),
 42+ 'Argument check error' => array(
 43+ 'string.byte()',
 44+ $xperr
 45+ ),
 46+ 'Protected infinite recursion' => array(
 47+ 'function foo() foo() end foo()',
 48+ $xperr
 49+ ),
 50+ 'Infinite recursion in handler' => array(
 51+ 'error("x")',
 52+ 'function foo() foo() end foo()'
 53+ ),
 54+ 'Protected infinite loop' => array(
 55+ 'while true do end',
 56+ $xperr,
 57+ ),
 58+ 'Infinite loop in handler' => array(
 59+ 'error("x")',
 60+ 'while true do end',
 61+ ),
 62+ 'Out of memory in handler' => array(
 63+ 'error("x")',
 64+ 'string.rep("x", 1000000)'
 65+ ),
 66+);
 67+
 68+$sandbox = new LuaSandbox;
 69+$sandbox->loadString( $lua )->call();
 70+$sandbox->setCPULimit( 0.25 );
 71+$sandbox->setMemoryLimit( 100000 );
 72+
 73+foreach ( $tests as $desc => $info ) {
 74+ $sandbox = new LuaSandbox;
 75+ $sandbox->loadString( $lua )->call();
 76+ $sandbox->setCPULimit( 0.25 );
 77+ $sandbox->setMemoryLimit( 100000 );
 78+ echo "$desc: ";
 79+ list( $code, $errorCode ) = $info;
 80+ $func = $sandbox->loadString( $code );
 81+ $errorCode = "return function(msg) $errorCode end";
 82+ $ret = $sandbox->loadString( $errorCode )->call();
 83+ $errorFunc = $ret[0];
 84+
 85+ try {
 86+ print implode("\n",
 87+ $sandbox->callFunction( 'xpcall_test', $func, $errorFunc ) ) . "\n";
 88+ } catch ( LuaSandboxError $e ) {
 89+ echo "LuaSandboxError: " . $e->getMessage() . "\n";
 90+ }
 91+}
 92+--EXPECT--
 93+Normal: success
 94+User error: xp: [string ""]:1: runtime error
 95+Error in error handler: LuaSandboxError: [string ""]:1: error in handler
 96+Unconvertible error in error handler: LuaSandboxError: unknown error
 97+Numeric error in error handler: LuaSandboxError: [string ""]:1: 2
 98+Argument check error: xp: [string ""]:1: bad argument #1 to 'byte' (string expected, got no value)
 99+Protected infinite recursion: LuaSandboxError: not enough memory
 100+Infinite recursion in handler: LuaSandboxError: not enough memory
 101+Protected infinite loop: LuaSandboxError: The maximum execution time for this script was exceeded
 102+Infinite loop in handler: LuaSandboxError: The maximum execution time for this script was exceeded
 103+Out of memory in handler: LuaSandboxError: not enough memory
Index: trunk/php/luasandbox/tests/pcall.phpt
——@@ -0,0 +1,46 @@
 2+--TEST--
 3+pcall() catching various errors
 4+--FILE--
 5+<?php
 6+$sandbox = new LuaSandbox;
 7+$lua = <<<LUA
 8+ function pcall_test(f)
 9+ local status, msg
 10+ status, msg = pcall(f)
 11+ if not status then
 12+ return "Caught: " .. msg
 13+ else
 14+ return "success"
 15+ end
 16+ end
 17+LUA;
 18+
 19+$tests = array(
 20+ 'Normal' => 'return 1',
 21+ 'User error' => 'error("runtime error")',
 22+ 'Argument check error' => 'string.byte()',
 23+ 'Infinite recursion' => 'function foo() foo() end foo()',
 24+ 'Infinite loop (timeout)' => 'while true do end',
 25+ 'Out of memory' => 'string.rep("x", 1000000)'
 26+);
 27+
 28+$sandbox->loadString( $lua )->call();
 29+$sandbox->setCPULimit( 0.25 );
 30+$sandbox->setMemoryLimit( 100000 );
 31+
 32+foreach ( $tests as $desc => $code ) {
 33+ echo "$desc: ";
 34+ try {
 35+ print implode("\n",
 36+ $sandbox->callFunction( 'pcall_test', $sandbox->loadString( $code ) ) ) . "\n";
 37+ } catch ( LuaSandboxError $e ) {
 38+ echo "LuaSandboxError: " . $e->getMessage() . "\n";
 39+ }
 40+}
 41+--EXPECT--
 42+Normal: success
 43+User error: Caught: [string ""]:1: runtime error
 44+Argument check error: Caught: [string ""]:1: bad argument #1 to 'byte' (string expected, got no value)
 45+Infinite recursion: LuaSandboxError: not enough memory
 46+Infinite loop (timeout): LuaSandboxError: The maximum execution time for this script was exceeded
 47+Out of memory: LuaSandboxError: not enough memory
Index: trunk/php/luasandbox/library.c
——@@ -20,11 +20,13 @@
2121 static int luasandbox_base_tostring(lua_State * L);
2222 static int luasandbox_math_random(lua_State * L);
2323 static int luasandbox_math_randomseed(lua_State * L);
 24+static int luasandbox_base_pcall(lua_State * L);
 25+static int luasandbox_base_xpcall (lua_State *L);
2426
2527 /**
2628 * Allowed global variables. Omissions are:
27 - * * pcall, xpcall: Changing the protected environment won't work with our
28 - * current timeout method.
 29+ * * pcall, xpcall: We have our own versions which don't allow interception of
 30+ * timeout etc. errors.
2931 * * loadfile: insecure.
3032 * * load, loadstring: Probably creates a protected environment so has
3133 * the same problem as pcall. Also omitting these makes analysis of the
——@@ -32,7 +34,7 @@
3335 * * print: Not compatible with a sandbox environment
3436 * * tostring: Provides addresses of tables and functions, which provides an
3537 * easy ASLR workaround or heap address discovery mechanism for a memory
36 - * corruption exploit.
 38+ * corruption exploit. We have our own version.
3739 * * Any new or undocumented functions like newproxy.
3840 * * package: cpath, loadlib etc. are insecure.
3941 * * coroutine: Not useful for our application so unreviewed at present.
——@@ -104,10 +106,15 @@
105107 }
106108 }
107109
108 - // Install our own version of tostring
 110+ // Install our own version of tostring, pcall, xpcall
109111 lua_pushcfunction(L, luasandbox_base_tostring);
110112 lua_setglobal(L, "tostring");
 113+ lua_pushcfunction(L, luasandbox_base_pcall);
 114+ lua_setglobal(L, "pcall");
 115+ lua_pushcfunction(L, luasandbox_base_xpcall);
 116+ lua_setglobal(L, "xpcall");
111117
 118+
112119 // Remove string.dump: may expose private data
113120 lua_getglobal(L, "string");
114121 lua_pushnil(L);
——@@ -235,3 +242,105 @@
236243 }
237244 /* }}} */
238245
 246+/** {{{ luasandbox_lib_rethrow_fatal
 247+ *
 248+ * If the error on the top of the stack with the error return code given as a
 249+ * parameter is a fatal, rethrow the error. If the error is rethrown, the
 250+ * function does not return.
 251+ */
 252+static void luasandbox_lib_rethrow_fatal(lua_State * L, int status)
 253+{
 254+ switch (status) {
 255+ case 0:
 256+ // No error
 257+ return;
 258+ case LUA_ERRRUN:
 259+ // A runtime error which we can rethrow in a normal way
 260+ if (luasandbox_is_fatal(L, -1)) {
 261+ lua_error(L);
 262+ }
 263+ break;
 264+ case LUA_ERRMEM:
 265+ case LUA_ERRERR:
 266+ // Lua doesn't provide a public API for rethrowing these, so we
 267+ // have to convert them to our own fatal error type
 268+ if (!luasandbox_is_fatal(L, -1)) {
 269+ luasandbox_wrap_fatal(L);
 270+ }
 271+ lua_error(L);
 272+ break; // not reached
 273+ }
 274+}
 275+/* }}} */
 276+
 277+/** {{{ luasandbox_lib_error_wrapper
 278+ *
 279+ * Wrapper for the xpcall error function
 280+ */
 281+static int luasandbox_lib_error_wrapper(lua_State * L)
 282+{
 283+ int status;
 284+ luaL_checkany(L, 1);
 285+
 286+ // This function is only called from luaG_errormsg(), which will later
 287+ // unconditionally set the status code to LUA_ERRRUN, so we can assume
 288+ // that the error type is equivalent to LUA_ERRRUN.
 289+ if (luasandbox_is_fatal(L, 1)) {
 290+ // Just return to whatever called lua_pcall(), don't call the user
 291+ // function
 292+ return lua_gettop(L);
 293+ }
 294+
 295+ // Put the user error function at the bottom of the stack
 296+ lua_pushvalue(L, lua_upvalueindex(1));
 297+ lua_insert(L, 1);
 298+ // Call it, passing through the arguments to this function
 299+ status = lua_pcall(L, lua_gettop(L) - 1, LUA_MULTRET, 0);
 300+ if (status != 0) {
 301+ luasandbox_lib_rethrow_fatal(L, LUA_ERRERR);
 302+ }
 303+ return lua_gettop(L);
 304+}
 305+/* }}} */
 306+
 307+/** {{{ luasandbox_base_pcall
 308+ *
 309+ * This is our implementation of the Lua function pcall(). It allows Lua code
 310+ * to handle its own errors, but forces internal errors to be rethrown.
 311+ */
 312+static int luasandbox_base_pcall(lua_State * L)
 313+{
 314+ int status;
 315+ luaL_checkany(L, 1);
 316+ status = lua_pcall(L, lua_gettop(L) - 1, LUA_MULTRET, 0);
 317+ luasandbox_lib_rethrow_fatal(L, status);
 318+ lua_pushboolean(L, (status == 0));
 319+ lua_insert(L, 1);
 320+ return lua_gettop(L); // return status + all results
 321+}
 322+/* }}} */
 323+
 324+/** {{{ luasandbox_base_xpcall
 325+ *
 326+ * This is our implementation of the Lua function xpcall(). It allows Lua code
 327+ * to handle its own errors, but forces internal errors to be rethrown.
 328+ */
 329+static int luasandbox_base_xpcall (lua_State *L)
 330+{
 331+ int status;
 332+ luaL_checkany(L, 2);
 333+ lua_settop(L, 2);
 334+
 335+ // We wrap the error function in a C closure. The error function already
 336+ // happens to be at the top of the stack, so we don't need to push it before
 337+ // calling lua_pushcfunction to make it an upvalue
 338+ lua_pushcclosure(L, luasandbox_lib_error_wrapper, 1);
 339+ lua_insert(L, 1); // put error function under function to be called
 340+
 341+ status = lua_pcall(L, 0, LUA_MULTRET, 1);
 342+ luasandbox_lib_rethrow_fatal(L, status);
 343+ lua_pushboolean(L, (status == 0));
 344+ lua_replace(L, 1);
 345+ return lua_gettop(L); // return status + all results
 346+}
 347+/* }}} */

Status & tagging log