Protect against server or CSM messing with the settings formspec

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 = {}

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,

src/client/game.cpp

@@ -646,7 +646,8 @@ private:
 	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;

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(),

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();

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