From c47edfb68f4dc65aa9e2a5c2be8ab0a61d67e8ae Mon Sep 17 00:00:00 2001 From: grorp Date: Wed, 1 Jan 2025 23:19:57 +0100 Subject: [PATCH] Protect against server or CSM messing with the settings formspec --- builtin/common/settings/dlg_settings.lua | 12 ++++++++++-- src/client/clientevent.h | 3 ++- src/client/game.cpp | 19 +++++++++++++++---- src/client/game_formspec.cpp | 19 ++++++++++++++++--- src/client/game_formspec.h | 3 ++- src/script/lua_api/l_client_common.cpp | 5 ++++- 6 files changed, 49 insertions(+), 12 deletions(-) diff --git a/builtin/common/settings/dlg_settings.lua b/builtin/common/settings/dlg_settings.lua index d8ed856f89b4b..96b9e57a4a5b9 100644 --- a/builtin/common/settings/dlg_settings.lua +++ b/builtin/common/settings/dlg_settings.lua @@ -671,7 +671,8 @@ local function buttonhandler(this, fields) dialogdata.rightscroll = core.explode_scrollbar_event(fields.rightscroll).value or dialogdata.rightscroll dialogdata.query = fields.search_query - if fields.back then + -- "quit" is for the pause menu env + if fields.back or fields.quit then this:delete() return true end @@ -782,14 +783,21 @@ else core.register_on_formspec_input(function(formname, fields) if dialog and formname == "__builtin:settings" then - -- dialog is re-checked since the buttonhandler may have closed it + -- buttonhandler returning true means we should update the formspec. + -- dialog is re-checked since the buttonhandler may have closed it. if buttonhandler(dialog, fields) and dialog then core.show_formspec("__builtin:settings", get_formspec(dialog.data)) end + return true end end) core.open_settings = function() + -- this assert is here to make sure we catch all cases of closing the dialog. + -- this is also a little additional protection against processing input + -- we shouldn't process (e.g. coming from server-sent formspecs). + assert(not dialog) + load() dialog = {} dialog.data = {} diff --git a/src/client/clientevent.h b/src/client/clientevent.h index 297124b30ecc0..f21b8220f1ef1 100644 --- a/src/client/clientevent.h +++ b/src/client/clientevent.h @@ -22,7 +22,8 @@ enum ClientEventType : u8 CE_PLAYER_FORCE_MOVE, CE_DEATHSCREEN_LEGACY, CE_SHOW_FORMSPEC, - CE_SHOW_LOCAL_FORMSPEC, + CE_SHOW_CSM_FORMSPEC, + CE_SHOW_PAUSE_MENU_FORMSPEC, CE_SPAWN_PARTICLE, CE_ADD_PARTICLESPAWNER, CE_DELETE_PARTICLESPAWNER, diff --git a/src/client/game.cpp b/src/client/game.cpp index c5f67a7453f17..5603e52864cb2 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -646,7 +646,8 @@ class Game { void handleClientEvent_PlayerForceMove(ClientEvent *event, CameraOrientation *cam); void handleClientEvent_DeathscreenLegacy(ClientEvent *event, CameraOrientation *cam); void handleClientEvent_ShowFormSpec(ClientEvent *event, CameraOrientation *cam); - void handleClientEvent_ShowLocalFormSpec(ClientEvent *event, CameraOrientation *cam); + void handleClientEvent_ShowCSMFormSpec(ClientEvent *event, CameraOrientation *cam); + void handleClientEvent_ShowPauseMenuFormSpec(ClientEvent *event, CameraOrientation *cam); void handleClientEvent_HandleParticleEvent(ClientEvent *event, CameraOrientation *cam); void handleClientEvent_HudAdd(ClientEvent *event, CameraOrientation *cam); @@ -2552,7 +2553,8 @@ const ClientEventHandler Game::clientEventHandler[CLIENTEVENT_MAX] = { {&Game::handleClientEvent_PlayerForceMove}, {&Game::handleClientEvent_DeathscreenLegacy}, {&Game::handleClientEvent_ShowFormSpec}, - {&Game::handleClientEvent_ShowLocalFormSpec}, + {&Game::handleClientEvent_ShowCSMFormSpec}, + {&Game::handleClientEvent_ShowPauseMenuFormSpec}, {&Game::handleClientEvent_HandleParticleEvent}, {&Game::handleClientEvent_HandleParticleEvent}, {&Game::handleClientEvent_HandleParticleEvent}, @@ -2620,9 +2622,18 @@ void Game::handleClientEvent_ShowFormSpec(ClientEvent *event, CameraOrientation delete event->show_formspec.formname; } -void Game::handleClientEvent_ShowLocalFormSpec(ClientEvent *event, CameraOrientation *cam) +void Game::handleClientEvent_ShowCSMFormSpec(ClientEvent *event, CameraOrientation *cam) { - m_game_formspec.showLocalFormSpec(*event->show_formspec.formspec, + m_game_formspec.showCSMFormSpec(*event->show_formspec.formspec, + *event->show_formspec.formname); + + delete event->show_formspec.formspec; + delete event->show_formspec.formname; +} + +void Game::handleClientEvent_ShowPauseMenuFormSpec(ClientEvent *event, CameraOrientation *cam) +{ + m_game_formspec.showPauseMenuFormSpec(*event->show_formspec.formspec, *event->show_formspec.formname); delete event->show_formspec.formspec; diff --git a/src/client/game_formspec.cpp b/src/client/game_formspec.cpp index 1417b1fc5ae8d..a6bc884c0adbd 100644 --- a/src/client/game_formspec.cpp +++ b/src/client/game_formspec.cpp @@ -228,16 +228,29 @@ void GameFormSpec::showFormSpec(const std::string &formspec, const std::string & } } -void GameFormSpec::showLocalFormSpec(const std::string &formspec, const std::string &formname) +void GameFormSpec::showCSMFormSpec(const std::string &formspec, const std::string &formname) { FormspecFormSource *fs_src = new FormspecFormSource(formspec); LocalFormspecHandler *txt_dst = - new LocalFormspecHandler(formname, m_client, m_pause_script.get()); + new LocalFormspecHandler(formname, m_client, nullptr); GUIFormSpecMenu::create(m_formspec, m_client, m_rendering_engine->get_gui_env(), &m_input->joystick, fs_src, txt_dst, m_client->getFormspecPrepend(), m_client->getSoundManager()); } +void GameFormSpec::showPauseMenuFormSpec(const std::string &formspec, const std::string &formname) +{ + // Neither CSM nor the server must be allowed to mess with the settings formspec, + // it's a trusted context like the mainmenu. + FormspecFormSource *fs_src = new FormspecFormSource(formspec); + LocalFormspecHandler *txt_dst = + new LocalFormspecHandler(formname, nullptr, m_pause_script.get()); + GUIFormSpecMenu::create(m_formspec, m_client, m_rendering_engine->get_gui_env(), + // Ignore formspec prepend. + &m_input->joystick, fs_src, txt_dst, "", + m_client->getSoundManager()); +} + void GameFormSpec::showNodeFormspec(const std::string &formspec, const v3s16 &nodepos) { infostream << "Launching custom inventory view" << std::endl; @@ -417,7 +430,7 @@ void GameFormSpec::showDeathFormspecLegacy() /* Note: FormspecFormSource and LocalFormspecHandler * * are deleted by guiFormSpecMenu */ FormspecFormSource *fs_src = new FormspecFormSource(formspec_str); - LocalFormspecHandler *txt_dst = new LocalFormspecHandler("MT_DEATH_SCREEN", m_client, m_pause_script.get()); + LocalFormspecHandler *txt_dst = new LocalFormspecHandler("MT_DEATH_SCREEN", m_client, nullptr); GUIFormSpecMenu::create(m_formspec, m_client, m_rendering_engine->get_gui_env(), &m_input->joystick, fs_src, txt_dst, m_client->getFormspecPrepend(), diff --git a/src/client/game_formspec.h b/src/client/game_formspec.h index 756e93e024b6f..fd69e103119ca 100644 --- a/src/client/game_formspec.h +++ b/src/client/game_formspec.h @@ -36,7 +36,8 @@ struct GameFormSpec ~GameFormSpec(); void showFormSpec(const std::string &formspec, const std::string &formname); - void showLocalFormSpec(const std::string &formspec, const std::string &formname); + void showCSMFormSpec(const std::string &formspec, const std::string &formname); + void showPauseMenuFormSpec(const std::string &formspec, const std::string &formname); void showNodeFormspec(const std::string &formspec, const v3s16 &nodepos); void showPlayerInventory(); void showDeathFormspecLegacy(); diff --git a/src/script/lua_api/l_client_common.cpp b/src/script/lua_api/l_client_common.cpp index 6e886e1059b8e..910b8eb638db9 100644 --- a/src/script/lua_api/l_client_common.cpp +++ b/src/script/lua_api/l_client_common.cpp @@ -6,6 +6,7 @@ #include "client/client.h" #include "client/clientevent.h" +#include "cpp_api/s_base.h" #include "lua_api/l_internal.h" // show_formspec(formspec) @@ -15,7 +16,9 @@ int ModApiClientCommon::l_show_formspec(lua_State *L) return 0; ClientEvent *event = new ClientEvent(); - event->type = CE_SHOW_LOCAL_FORMSPEC; + event->type = getScriptApiBase(L)->getType() == ScriptingType::PauseMenu + ? CE_SHOW_PAUSE_MENU_FORMSPEC + : CE_SHOW_CSM_FORMSPEC; event->show_formspec.formname = new std::string(luaL_checkstring(L, 1)); event->show_formspec.formspec = new std::string(luaL_checkstring(L, 2)); getClient(L)->pushToEventQueue(event);