Fix potential use-after-free with item metadata (#12729)

This fixes a use-after-free bug in the case where itemstack metadata is accessed after the itemstack has been garbage-collected.

src/script/lua_api/l_item.cpp

@@ -34,7 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 int LuaItemStack::gc_object(lua_State *L)
 {
 	LuaItemStack *o = *(LuaItemStack **)(lua_touserdata(L, 1));
-	delete o;
+	o->drop();
 	return 0;
 }
 
@@ -152,7 +152,7 @@ int LuaItemStack::l_get_meta(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
 	LuaItemStack *o = checkobject(L, 1);
-	ItemStackMetaRef::create(L, &o->m_stack);
+	ItemStackMetaRef::create(L, o);
 	return 1;
 }
 
@@ -438,15 +438,6 @@ LuaItemStack::LuaItemStack(const ItemStack &item):
 {
 }
 
-const ItemStack& LuaItemStack::getItem() const
-{
-	return m_stack;
-}
-ItemStack& LuaItemStack::getItem()
-{
-	return m_stack;
-}
-
 // LuaItemStack(itemstack or itemstring or table or nil)
 // Creates an LuaItemStack and leaves it on top of stack
 int LuaItemStack::create_object(lua_State *L)

src/script/lua_api/l_item.h

@@ -21,11 +21,15 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "lua_api/l_base.h"
 #include "inventory.h"  // ItemStack
+#include "util/pointer.h"
 
-class LuaItemStack : public ModApiBase {
+class LuaItemStack : public ModApiBase, public IntrusiveReferenceCounted {
 private:
 	ItemStack m_stack;
 
+	LuaItemStack(const ItemStack &item);
+	~LuaItemStack() = default;
+
 	static const char className[];
 	static const luaL_Reg methods[];
 
@@ -138,11 +142,10 @@ private:
 	static int l_peek_item(lua_State *L);
 
 public:
-	LuaItemStack(const ItemStack &item);
+	DISABLE_CLASS_COPY(LuaItemStack)
-	~LuaItemStack() = default;
 
-	const ItemStack& getItem() const;
+	inline const ItemStack& getItem() const { return m_stack; }
-	ItemStack& getItem();
+	inline ItemStack& getItem() { return m_stack; }
 
 	// LuaItemStack(itemstack or itemstring or table or nil)
 	// Creates an LuaItemStack and leaves it on top of stack

src/script/lua_api/l_itemstackmeta.cpp

@@ -38,12 +38,12 @@ ItemStackMetaRef* ItemStackMetaRef::checkobject(lua_State *L, int narg)
 
 Metadata* ItemStackMetaRef::getmeta(bool auto_create)
 {
-	return &istack->metadata;
+	return &istack->getItem().metadata;
 }
 
 void ItemStackMetaRef::clearMeta()
 {
-	istack->metadata.clear();
+	istack->getItem().metadata.clear();
 }
 
 void ItemStackMetaRef::reportMetadataChange(const std::string *name)
@@ -67,6 +67,16 @@ int ItemStackMetaRef::l_set_tool_capabilities(lua_State *L)
 	return 0;
 }
 
+ItemStackMetaRef::ItemStackMetaRef(LuaItemStack *istack): istack(istack)
+{
+	istack->grab();
+}
+
+ItemStackMetaRef::~ItemStackMetaRef()
+{
+	istack->drop();
+}
+
 // garbage collector
 int ItemStackMetaRef::gc_object(lua_State *L) {
 	ItemStackMetaRef *o = *(ItemStackMetaRef **)(lua_touserdata(L, 1));
@@ -76,7 +86,7 @@ int ItemStackMetaRef::gc_object(lua_State *L) {
 
 // Creates an NodeMetaRef and leaves it on top of stack
 // Not callable from Lua; all references are created on the C side.
-void ItemStackMetaRef::create(lua_State *L, ItemStack *istack)
+void ItemStackMetaRef::create(lua_State *L, LuaItemStack *istack)
 {
 	ItemStackMetaRef *o = new ItemStackMetaRef(istack);
 	//infostream<<"NodeMetaRef::create: o="<<o<<std::endl;

src/script/lua_api/l_itemstackmeta.h

@@ -23,13 +23,13 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "lua_api/l_base.h"
 #include "lua_api/l_metadata.h"
+#include "lua_api/l_item.h"
 #include "irrlichttypes_bloated.h"
-#include "inventory.h"
 
 class ItemStackMetaRef : public MetaDataRef
 {
 private:
-	ItemStack *istack = nullptr;
+	LuaItemStack *istack;
 
 	static const char className[];
 	static const luaL_Reg methods[];
@@ -44,12 +44,12 @@ private:
 
 	void setToolCapabilities(const ToolCapabilities &caps)
 	{
-		istack->metadata.setToolCapabilities(caps);
+		istack->getItem().metadata.setToolCapabilities(caps);
 	}
 
 	void clearToolCapabilities()
 	{
-		istack->metadata.clearToolCapabilities();
+		istack->getItem().metadata.clearToolCapabilities();
 	}
 
 	// Exported functions
@@ -58,12 +58,15 @@ private:
 	// garbage collector
 	static int gc_object(lua_State *L);
 public:
-	ItemStackMetaRef(ItemStack *istack): istack(istack) {}
+	// takes a reference
-	~ItemStackMetaRef() = default;
+	ItemStackMetaRef(LuaItemStack *istack);
+	~ItemStackMetaRef();
+
+	DISABLE_CLASS_COPY(ItemStackMetaRef)
 
 	// Creates an ItemStackMetaRef and leaves it on top of stack
 	// Not callable from Lua; all references are created on the C side.
-	static void create(lua_State *L, ItemStack *istack);
+	static void create(lua_State *L, LuaItemStack *istack);
 
 	static void Register(lua_State *L);
 };

src/util/pointer.h

@@ -257,3 +257,17 @@ private:
 	unsigned int *refcount;
 };
 
+// This class is not thread-safe!
+class IntrusiveReferenceCounted {
+public:
+	IntrusiveReferenceCounted() = default;
+	virtual ~IntrusiveReferenceCounted() = default;
+	void grab() noexcept { ++m_refcount; }
+	void drop() noexcept { if (--m_refcount == 0) delete this; }
+
+	// Preserve own reference count.
+	IntrusiveReferenceCounted(const IntrusiveReferenceCounted &) {}
+	IntrusiveReferenceCounted &operator=(const IntrusiveReferenceCounted &) { return *this; }
+private:
+	u32 m_refcount = 1;
+};