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.
This commit is contained in:
Jude Melton-Houghton 2022-09-11 13:28:37 -04:00 committed by sfan5
parent 129aef758e
commit f8bb0cd3d1
5 changed files with 47 additions and 26 deletions

@ -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)

@ -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

@ -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;

@ -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);
};

@ -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;
};