Make tools (and thus elements) read-only in some contexts

Namely, in Select and Click tool callbacks, which are otherwise interface contexts and allow changing tools. This may cause use-after-free but it's much more likely to cause stack overflows. Importantly, the setter usage of ui.activeTool is considered to change tool state because it invokes Select callbacks.
This commit is contained in:
Tamás Bálint Misius
2025-05-06 18:13:31 +02:00
parent b464c3aa3f
commit f65c4ee4e1
6 changed files with 23 additions and 10 deletions

View File

@@ -12,6 +12,7 @@ enum EventTraits : uint32_t
eventTraitInterface = UINT32_C(0x00000008), eventTraitInterface = UINT32_C(0x00000008),
eventTraitInterfaceGraphics = UINT32_C(0x00000010), eventTraitInterfaceGraphics = UINT32_C(0x00000010),
eventTraitConstSim = UINT32_C(0x00000020), eventTraitConstSim = UINT32_C(0x00000020),
eventTraitConstTools = UINT32_C(0x00000040),
}; };
constexpr EventTraits operator |(EventTraits lhs, EventTraits rhs) constexpr EventTraits operator |(EventTraits lhs, EventTraits rhs)
{ {

View File

@@ -324,7 +324,7 @@ static bool luaCtypeDrawWrapper(CTYPEDRAW_FUNC_ARGS)
static int allocate(lua_State *L) static int allocate(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
luaL_checktype(L, 1, LUA_TSTRING); luaL_checktype(L, 1, LUA_TSTRING);
luaL_checktype(L, 2, LUA_TSTRING); luaL_checktype(L, 2, LUA_TSTRING);
auto group = tpt_lua_toByteString(L, 1).ToUpper(); auto group = tpt_lua_toByteString(L, 1).ToUpper();
@@ -422,7 +422,7 @@ static int element(lua_State *L)
if (lua_gettop(L) > 1) if (lua_gettop(L) > 1)
{ {
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
{ {
auto &sd = SimulationData::Ref(); auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx); std::unique_lock lk(sd.elementGraphicsMx);
@@ -573,7 +573,7 @@ static int property(lua_State *L)
if (lua_gettop(L) > 2) if (lua_gettop(L) > 2)
{ {
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
auto &sd = SimulationData::Ref(); auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx); std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements; auto &elements = sd.elements;
@@ -727,7 +727,7 @@ static int property(lua_State *L)
static int ffree(lua_State *L) static int ffree(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
int id = luaL_checkinteger(L, 1); int id = luaL_checkinteger(L, 1);
ByteString identifier; ByteString identifier;
@@ -773,7 +773,7 @@ static int exists(lua_State *L)
static int loadDefault(lua_State *L) static int loadDefault(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
auto &sd = SimulationData::Ref(); auto &sd = SimulationData::Ref();
std::unique_lock lk(sd.elementGraphicsMx); std::unique_lock lk(sd.elementGraphicsMx);
auto &elements = sd.elements; auto &elements = sd.elements;

View File

@@ -256,6 +256,7 @@ static int activeTool(lua_State *L)
tpt_lua_pushByteString(L, lsi->gameModel->GetActiveTool(index)->Identifier); tpt_lua_pushByteString(L, lsi->gameModel->GetActiveTool(index)->Identifier);
return 1; return 1;
} }
lsi->AssertMutableToolsEvent();
auto identifier = tpt_lua_checkByteString(L, 2); auto identifier = tpt_lua_checkByteString(L, 2);
auto *tool = lsi->gameModel->GetToolFromIdentifier(identifier); auto *tool = lsi->gameModel->GetToolFromIdentifier(identifier);
if (!tool) if (!tool)

View File

@@ -900,6 +900,7 @@ void LuaScriptInterface::AssertInterfaceEvent()
luaL_error(L, "this functionality is restricted to interface events"); luaL_error(L, "this functionality is restricted to interface events");
} }
} }
void LuaScriptInterface::AssertMutableSimEvent() void LuaScriptInterface::AssertMutableSimEvent()
{ {
if (eventTraits & eventTraitConstSim) if (eventTraits & eventTraitConstSim)
@@ -907,3 +908,12 @@ void LuaScriptInterface::AssertMutableSimEvent()
luaL_error(L, "this functionality is restricted to mutable simulation events"); luaL_error(L, "this functionality is restricted to mutable simulation events");
} }
} }
void LuaScriptInterface::AssertMutableToolsEvent()
{
AssertInterfaceEvent();
if (eventTraits & eventTraitConstTools)
{
luaL_error(L, "this functionality is restricted to mutable tool events");
}
}

View File

@@ -153,6 +153,7 @@ public:
void AssertInterfaceEvent(); void AssertInterfaceEvent();
void AssertMutableSimEvent(); void AssertMutableSimEvent();
void AssertMutableToolsEvent();
friend class CommandInterface; friend class CommandInterface;
}; };

View File

@@ -7,7 +7,7 @@
static int allocate(lua_State *L) static int allocate(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
luaL_checktype(L, 1, LUA_TSTRING); luaL_checktype(L, 1, LUA_TSTRING);
luaL_checktype(L, 2, LUA_TSTRING); luaL_checktype(L, 2, LUA_TSTRING);
auto group = tpt_lua_toByteString(L, 1).ToUpper(); auto group = tpt_lua_toByteString(L, 1).ToUpper();
@@ -51,7 +51,7 @@ static bool IsCustom(int index)
static int ffree(lua_State *L) static int ffree(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
int index = luaL_checkinteger(L, 1); int index = luaL_checkinteger(L, 1);
auto *tool = lsi->gameModel->GetToolByIndex(index); auto *tool = lsi->gameModel->GetToolByIndex(index);
if (!tool) if (!tool)
@@ -127,7 +127,7 @@ static void luaClickWrapper(SimTool *tool, Simulation *sim, const Brush &brush,
lua_pushboolean(L, tool->shiftBehaviour); lua_pushboolean(L, tool->shiftBehaviour);
lua_pushboolean(L, tool->ctrlBehaviour); lua_pushboolean(L, tool->ctrlBehaviour);
lua_pushboolean(L, tool->altBehaviour); lua_pushboolean(L, tool->altBehaviour);
if (tpt_lua_pcall(L, 7, 0, 0, eventTraitInterface)) if (tpt_lua_pcall(L, 7, 0, 0, eventTraitInterface | eventTraitConstTools))
{ {
lsi->Log(CommandInterface::LogError, "In click func: " + LuaGetError()); lsi->Log(CommandInterface::LogError, "In click func: " + LuaGetError());
lua_pop(L, 1); lua_pop(L, 1);
@@ -271,7 +271,7 @@ static void luaSelectWrapper(SimTool *tool, int toolSelection)
{ {
lua_rawgeti(L, LUA_REGISTRYINDEX, customTools[index].select); lua_rawgeti(L, LUA_REGISTRYINDEX, customTools[index].select);
lua_pushinteger(L, toolSelection); lua_pushinteger(L, toolSelection);
if (tpt_lua_pcall(L, 1, 0, 0, eventTraitInterface)) if (tpt_lua_pcall(L, 1, 0, 0, eventTraitInterface | eventTraitConstTools))
{ {
lsi->Log(CommandInterface::LogError, "In select func: " + LuaGetError()); lsi->Log(CommandInterface::LogError, "In select func: " + LuaGetError());
lua_pop(L, 1); lua_pop(L, 1);
@@ -287,7 +287,7 @@ struct DependentFalse : std::false_type
static int property(lua_State *L) static int property(lua_State *L)
{ {
auto *lsi = GetLSI(); auto *lsi = GetLSI();
lsi->AssertInterfaceEvent(); lsi->AssertMutableToolsEvent();
int index = luaL_checkinteger(L, 1); int index = luaL_checkinteger(L, 1);
auto *tool = lsi->gameModel->GetToolByIndex(index); auto *tool = lsi->gameModel->GetToolByIndex(index);
if (!tool) if (!tool)