Fix Lua type confusion bug

The only secure and robust way to check that a userdata is of the
expected type is to check its metatable.  Userdata metatables are not
changeable by Lua code without the debug library, so if the metatable is
a certain table, only C code could have made it so.  GLib type checking
functions are _not_ a robust or secure way to check that a block of
memory is a specific type of GObject, because Lua code could cause a
type confusion bug and potentially use it to forge pointers.

modules/module-lua-scripting/wplua/boxed.c

@@ -26,9 +26,9 @@ find_method_in_luaL_Reg (luaL_Reg *reg, const gchar *method)
 static int
 _wplua_gboxed___index (lua_State *L)
 {
-  luaL_argcheck (L, wplua_isboxed (L, 1, G_TYPE_BOXED), 1,
+  GValue *obj_v = _wplua_togvalue_userdata_named (L, 1, G_TYPE_BOXED, "GBoxed");
+  luaL_argcheck (L, obj_v != NULL, 1,
       "expected userdata storing GValue<GBoxed>");
-  GValue *obj_v = lua_touserdata (L, 1);
   const gchar *key = luaL_checkstring (L, 2);
   GType type = G_VALUE_TYPE (obj_v);
   lua_CFunction func = NULL;
@@ -56,17 +56,24 @@ _wplua_gboxed___index (lua_State *L)
   return 0;
 }
 
+static int
+_wplua_gboxed___eq (lua_State *L)
+{
+  return _wplua_gvalue_userdata___eq_impl (L, "GBoxed");
+}
+
 void
 _wplua_init_gboxed (lua_State *L)
 {
   static const luaL_Reg gboxed_meta[] = {
     { "__gc", _wplua_gvalue_userdata___gc },
-    { "__eq", _wplua_gvalue_userdata___eq },
+    { "__eq", _wplua_gboxed___eq },
     { "__index", _wplua_gboxed___index },
     { NULL, NULL }
   };
 
-  luaL_newmetatable (L, "GBoxed");
+  if (!luaL_newmetatable (L, "GBoxed"))
+    g_error ("Metatable with key GBoxed in the registry already exists?");
   luaL_setfuncs (L, gboxed_meta, 0);
   lua_pop (L, 1);
 }
@@ -79,31 +86,31 @@ wplua_pushboxed (lua_State * L, GType type, gpointer object)
   GValue *v = _wplua_pushgvalue_userdata (L, type);
   wp_trace_boxed (type, object, "pushing to Lua, v=%p", v);
   g_value_take_boxed (v, object);
-
-  luaL_getmetatable (L, "GBoxed");
-  lua_setmetatable (L, -2);
 }
 
 gpointer
 wplua_toboxed (lua_State *L, int idx)
 {
-  g_return_val_if_fail (_wplua_isgvalue_userdata (L, idx, G_TYPE_BOXED), NULL);
-  return g_value_get_boxed ((GValue *) lua_touserdata (L, idx));
+  GValue *v = _wplua_togvalue_userdata_named (L, idx, G_TYPE_BOXED, "GBoxed");
+
+  g_return_val_if_fail (v, NULL);
+  return g_value_get_boxed (v);
 }
 
 gpointer
 wplua_checkboxed (lua_State *L, int idx, GType type)
 {
-  if (G_UNLIKELY (!_wplua_isgvalue_userdata (L, idx, type))) {
+  GValue *v = _wplua_togvalue_userdata_named (L, idx, type, "GBoxed");
+  if (v == NULL) {
     wp_critical ("expected userdata storing GValue<%s>", g_type_name (type));
     luaL_argerror (L, idx, "expected userdata storing GValue<GBoxed>");
   }
-  return g_value_get_boxed ((GValue *) lua_touserdata (L, idx));
+  return g_value_get_boxed (v);
 }
 
 gboolean
 wplua_isboxed (lua_State *L, int idx, GType type)
 {
-  if (!g_type_is_a (type, G_TYPE_BOXED)) return FALSE;
-  return _wplua_isgvalue_userdata (L, idx, type);
+  return g_type_is_a (type, G_TYPE_BOXED) &&
+         _wplua_togvalue_userdata_named (L, idx, type, "GBoxed") != NULL;
 }

modules/module-lua-scripting/wplua/object.c

@@ -191,19 +191,26 @@ _wplua_gobject__tostring (lua_State *L)
   return 1;
 }
 
+static int
+_wplua_gobject___eq (lua_State *L)
+{
+  return _wplua_gvalue_userdata___eq_impl (L, "GObject");
+}
+
 void
 _wplua_init_gobject (lua_State *L)
 {
   static const luaL_Reg gobject_meta[] = {
     { "__gc", _wplua_gvalue_userdata___gc },
-    { "__eq", _wplua_gvalue_userdata___eq },
+    { "__eq", _wplua_gobject___eq },
     { "__index", _wplua_gobject___index },
     { "__newindex", _wplua_gobject___newindex },
     { "__tostring", _wplua_gobject__tostring },
     { NULL, NULL }
   };
 
-  luaL_newmetatable (L, "GObject");
+  if (!luaL_newmetatable (L, "GObject"))
+    g_error ("Metatable with key GObject in the registry already exists?");
   luaL_setfuncs (L, gobject_meta, 0);
   lua_pop (L, 1);
 }
@@ -216,31 +223,31 @@ wplua_pushobject (lua_State * L, gpointer object)
   GValue *v = _wplua_pushgvalue_userdata (L, G_TYPE_FROM_INSTANCE (object));
   wp_trace_object (object, "pushing to Lua, v=%p", v);
   g_value_take_object (v, object);
-
-  luaL_getmetatable (L, "GObject");
-  lua_setmetatable (L, -2);
 }
 
 gpointer
 wplua_toobject (lua_State *L, int idx)
 {
-  g_return_val_if_fail (_wplua_isgvalue_userdata (L, idx, G_TYPE_OBJECT), NULL);
-  return g_value_get_object ((GValue *) lua_touserdata (L, idx));
+  GValue *v = _wplua_togvalue_userdata_named (L, idx, G_TYPE_OBJECT, "GObject");
+
+  g_return_val_if_fail (v, NULL);
+  return g_value_get_object (v);
 }
 
 gpointer
 wplua_checkobject (lua_State *L, int idx, GType type)
 {
-  if (G_UNLIKELY (!_wplua_isgvalue_userdata (L, idx, type))) {
+  GValue *v = _wplua_togvalue_userdata_named (L, idx, type, "GObject");
+  if (v == NULL) {
     wp_critical ("expected userdata storing GValue<%s>", g_type_name (type));
     luaL_argerror (L, idx, "expected userdata storing GValue<GObject>");
   }
-  return g_value_get_object ((GValue *) lua_touserdata (L, idx));
+  return g_value_get_object (v);
 }
 
 gboolean
 wplua_isobject (lua_State *L, int idx, GType type)
 {
-  if (!g_type_is_a (type, G_TYPE_OBJECT)) return FALSE;
-  return _wplua_isgvalue_userdata (L, idx, type);
+  return g_type_is_a (type, G_TYPE_OBJECT) &&
+         _wplua_togvalue_userdata_named (L, idx, type, "GObject") != NULL;
 }

modules/module-lua-scripting/wplua/private.h

@@ -27,10 +27,11 @@ void _wplua_init_gobject (lua_State *L);
 
 /* userdata.c */
 GValue * _wplua_pushgvalue_userdata (lua_State * L, GType type);
+GValue * _wplua_togvalue_userdata_named (lua_State *L, int idx, GType type, const char *table_name);
+GValue * _wplua_togvalue_userdata (lua_State *L, int idx, GType type);
 gboolean _wplua_isgvalue_userdata (lua_State *L, int idx, GType type);
-
+int _wplua_gvalue_userdata___eq_impl (lua_State *L, const char *type);
 int _wplua_gvalue_userdata___gc (lua_State *L);
-int _wplua_gvalue_userdata___eq (lua_State *L);
 
 /* wplua.c */
 int _wplua_pcall (lua_State *L, int nargs, int nret);

modules/module-lua-scripting/wplua/userdata.c

@@ -10,30 +10,69 @@
 #include "private.h"
 #include <wp/wp.h>
 
+static const char *
+_wplua_get_metatable_name (GType type)
+{
+  if (g_type_is_a (type, G_TYPE_BOXED))
+    return "GBoxed";
+  else if (g_type_is_a (type, G_TYPE_OBJECT))
+    return "GObject";
+  else
+    return NULL;
+}
+
 GValue *
 _wplua_pushgvalue_userdata (lua_State * L, GType type)
 {
-  GValue *v = lua_newuserdata (L, sizeof (GValue));
+  GValue *v;
+  const char *table_name = _wplua_get_metatable_name (type);
+
+  if (table_name == NULL)
+    g_error ("type passed to %s not boxed or object", __func__);
+
+  /* auxillary library can use 4 stack slots, plus 1 for userdata */
+  if (!lua_checkstack (L, 5))
+    g_error ("cannot grow Lua stack in %s", __func__);
+  v = lua_newuserdata (L, sizeof (GValue));
   memset (v, 0, sizeof (GValue));
   g_value_init (v, type);
+  luaL_getmetatable (L, table_name);
+  lua_setmetatable (L, -2);
   return v;
 }
 
+GValue *
+_wplua_togvalue_userdata_named (lua_State *L, int idx, GType type,
+                                const char *table_name)
+{
+  GValue *v;
+
+  /* auxillary library can use 4 stack slots */
+  if (!lua_checkstack (L, 4))
+    g_error ("cannot grow Lua stack in %s", __func__);
+  if (!(v = luaL_testudata (L, idx, table_name)))
+    return NULL;
+  /* if this triggers someone misused the debug library */
+  if (lua_rawlen (L, idx) != sizeof (GValue))
+    g_error ("Wrong length for userdata of type %s", table_name);
+  if (type != G_TYPE_NONE && !g_type_is_a (G_VALUE_TYPE (v), type))
+    return NULL;
+
+  return v;
+}
+
+GValue *
+_wplua_togvalue_userdata (lua_State *L, int idx, GType type)
+{
+  const char *table_name = _wplua_get_metatable_name (type);
+  return table_name == NULL ? NULL :
+         _wplua_togvalue_userdata_named (L, idx, type, table_name);
+}
+
 gboolean
 _wplua_isgvalue_userdata (lua_State *L, int idx, GType type)
 {
-  GValue *v;
-
-  if (!lua_isuserdata (L, idx))
-    return FALSE;
-  if (lua_rawlen (L, idx) != sizeof (GValue))
-    return FALSE;
-  if (!(v = lua_touserdata (L, idx)))
-    return FALSE;
-  if (type != G_TYPE_NONE && !g_type_is_a (G_VALUE_TYPE (v), type))
-    return FALSE;
-
-  return TRUE;
+  return _wplua_togvalue_userdata (L, idx, type) != NULL;
 }
 
 GType
@@ -62,12 +101,15 @@ _wplua_gvalue_userdata___gc (lua_State *L)
 }
 
 int
-_wplua_gvalue_userdata___eq (lua_State *L)
+_wplua_gvalue_userdata___eq_impl (lua_State *L, const char *type)
 {
-  if (_wplua_isgvalue_userdata (L, 1, G_TYPE_NONE) &&
-      _wplua_isgvalue_userdata (L, 2, G_TYPE_NONE)) {
-    GValue *v1 = lua_touserdata (L, 1);
-    GValue *v2 = lua_touserdata (L, 2);
+  /*
+   * First argument should always be a userdata.
+   * Second argument can be anything.
+   */
+  GValue *v1 = luaL_checkudata (L, 1, type);
+  GValue *v2 = luaL_testudata (L, 2, type);
+  if (v2 != NULL) {
     gpointer p1 = g_value_peek_pointer (v1);
     gpointer p2 = g_value_peek_pointer (v2);
     lua_pushboolean (L, (p1 == p2));