aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJude Melton-Houghton <jwmhjwmh@gmail.com>2022-09-11 13:28:37 -0400
committersfan5 <sfan5@live.de>2022-09-14 13:48:06 +0200
commitf8bb0cd3d1da9d2d9d8dffe78cd0fb651e16a8af (patch)
treee301b693df3b4b79302bbca1221887046da84d28
parent129aef758ece753e684aa494e5471045a996ac8f (diff)
downloadminetest-f8bb0cd3d1da9d2d9d8dffe78cd0fb651e16a8af.tar.gz
minetest-f8bb0cd3d1da9d2d9d8dffe78cd0fb651e16a8af.tar.bz2
minetest-f8bb0cd3d1da9d2d9d8dffe78cd0fb651e16a8af.zip
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.
-rw-r--r--src/script/lua_api/l_item.cpp13
-rw-r--r--src/script/lua_api/l_item.h13
-rw-r--r--src/script/lua_api/l_itemstackmeta.cpp16
-rw-r--r--src/script/lua_api/l_itemstackmeta.h17
-rw-r--r--src/util/pointer.h14
5 files changed, 47 insertions, 26 deletions
diff --git a/src/script/lua_api/l_item.cpp b/src/script/lua_api/l_item.cpp
index 13d046d00..05473f43d 100644
--- a/src/script/lua_api/l_item.cpp
+++ b/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)
diff --git a/src/script/lua_api/l_item.h b/src/script/lua_api/l_item.h
index a392555d2..72b1922dd 100644
--- a/src/script/lua_api/l_item.h
+++ b/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);
- ~LuaItemStack() = default;
+ DISABLE_CLASS_COPY(LuaItemStack)
- const ItemStack& getItem() const;
- ItemStack& getItem();
+ inline const ItemStack& getItem() const { return m_stack; }
+ inline ItemStack& getItem() { return m_stack; }
// LuaItemStack(itemstack or itemstring or table or nil)
// Creates an LuaItemStack and leaves it on top of stack
diff --git a/src/script/lua_api/l_itemstackmeta.cpp b/src/script/lua_api/l_itemstackmeta.cpp
index 739fb9221..c17bb8995 100644
--- a/src/script/lua_api/l_itemstackmeta.cpp
+++ b/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;
diff --git a/src/script/lua_api/l_itemstackmeta.h b/src/script/lua_api/l_itemstackmeta.h
index c3198be4f..68d2ba8fa 100644
--- a/src/script/lua_api/l_itemstackmeta.h
+++ b/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) {}
- ~ItemStackMetaRef() = default;
+ // takes a reference
+ 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);
};
diff --git a/src/util/pointer.h b/src/util/pointer.h
index b659cea0e..f4b70f822 100644
--- a/src/util/pointer.h
+++ b/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;
+};