From a972a54ea9f2e000d6054c66eaadf599a56ad5a9 Mon Sep 17 00:00:00 2001 From: uramer Date: Fri, 28 Jan 2022 21:35:05 +0100 Subject: [PATCH] Allow changing element root widget type, prevent use after free in script settings --- apps/openmw/mwgui/settingswindow.cpp | 20 +++++----- apps/openmw/mwgui/settingswindow.hpp | 1 + components/lua_ui/element.cpp | 60 ++++++++++++++++++++++++---- components/lua_ui/element.hpp | 7 +++- components/lua_ui/scriptsettings.cpp | 9 ++--- components/lua_ui/scriptsettings.hpp | 2 +- 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/apps/openmw/mwgui/settingswindow.cpp b/apps/openmw/mwgui/settingswindow.cpp index 567e603e53..3ea60b92bb 100644 --- a/apps/openmw/mwgui/settingswindow.cpp +++ b/apps/openmw/mwgui/settingswindow.cpp @@ -207,9 +207,9 @@ namespace MWGui } } - SettingsWindow::SettingsWindow() : - WindowBase("openmw_settings_window.layout"), - mKeyboardMode(true) + SettingsWindow::SettingsWindow() : WindowBase("openmw_settings_window.layout") + , mKeyboardMode(true) + , mCurrentPage(-1) { bool terrain = Settings::Manager::getBool("distant terrain", "Terrain"); const std::string widgetName = terrain ? "RenderingDistanceSlider" : "LargeRenderingDistanceSlider"; @@ -729,8 +729,8 @@ namespace MWGui void SettingsWindow::renderScriptSettings() { - while (mScriptView->getChildCount() > 0) - mScriptView->getChildAt(0)->detachFromWidget(); + LuaUi::attachToWidget(mCurrentPage); + mCurrentPage = -1; mScriptList->removeAllItems(); mScriptView->setCanvasSize({0, 0}); @@ -763,13 +763,13 @@ namespace MWGui void SettingsWindow::onScriptListSelection(MyGUI::Widget*, size_t index) { - while (mScriptView->getChildCount() > 0) - mScriptView->getChildAt(0)->detachFromWidget(); + if (mCurrentPage >= 0) + LuaUi::attachToWidget(mCurrentPage); + mCurrentPage = -1; if (index >= mScriptList->getItemCount()) return; - size_t scriptIndex = *mScriptList->getItemDataAt(index); - LuaUi::ScriptSettings script = LuaUi::scriptSettings()[scriptIndex]; - LuaUi::attachToWidget(script, mScriptView); + mCurrentPage = *mScriptList->getItemDataAt(index); + LuaUi::attachToWidget(mCurrentPage, mScriptView); MyGUI::IntSize canvasSize; if (mScriptView->getChildCount() > 0) canvasSize = mScriptView->getChildAt(0)->getSize(); diff --git a/apps/openmw/mwgui/settingswindow.hpp b/apps/openmw/mwgui/settingswindow.hpp index c7ba1e8ece..2db8f2e19c 100644 --- a/apps/openmw/mwgui/settingswindow.hpp +++ b/apps/openmw/mwgui/settingswindow.hpp @@ -49,6 +49,7 @@ namespace MWGui MyGUI::Widget* mScriptBox; MyGUI::ScrollView* mScriptView; MyGUI::EditBox* mScriptDisabled; + int mCurrentPage; void onTabChanged(MyGUI::TabControl* _sender, size_t index); void onOkButtonClicked(MyGUI::Widget* _sender); diff --git a/components/lua_ui/element.cpp b/components/lua_ui/element.cpp index c9e0fc309e..5baddb98d8 100644 --- a/components/lua_ui/element.cpp +++ b/components/lua_ui/element.cpp @@ -138,7 +138,7 @@ namespace LuaUi ext->setChildren(updateContent(ext->children(), layout.get(LayoutKeys::content))); } - void setLayer(WidgetExtension* ext, const sol::table& layout) + std::string setLayer(WidgetExtension* ext, const sol::table& layout) { MyGUI::ILayer* layerNode = ext->widget()->getLayer(); std::string currentLayer = layerNode ? layerNode->getName() : std::string(); @@ -149,15 +149,18 @@ namespace LuaUi { MyGUI::LayerManager::getInstance().attachToLayerNode(newLayer, ext->widget()); } + return newLayer; } std::map> Element::sAllElements; Element::Element(sol::table layout) - : mRoot{ nullptr } - , mLayout{ std::move(layout) } - , mUpdate{ false } - , mDestroy{ false } + : mRoot(nullptr) + , mAttachedTo(nullptr) + , mLayout(std::move(layout)) + , mLayer() + , mUpdate(false) + , mDestroy(false) {} @@ -174,7 +177,8 @@ namespace LuaUi if (!mRoot) { mRoot = createWidget(mLayout); - setLayer(mRoot, mLayout); + mLayer = setLayer(mRoot, mLayout); + updateAttachment(); } } @@ -182,8 +186,17 @@ namespace LuaUi { if (mRoot && mUpdate) { - updateWidget(mRoot, mLayout); - setLayer(mRoot, mLayout); + if (mRoot->widget()->getTypeName() != widgetType(mLayout)) + { + destroyWidget(mRoot); + mRoot = createWidget(mLayout); + } + else + { + updateWidget(mRoot, mLayout); + } + mLayer = setLayer(mRoot, mLayout); + updateAttachment(); } mUpdate = false; } @@ -195,4 +208,35 @@ namespace LuaUi mRoot = nullptr; sAllElements.erase(this); } + + void Element::attachToWidget(MyGUI::Widget* w) + { + if (mAttachedTo && w) + throw std::logic_error("A UI element can't be attached to two widgets at once"); + mAttachedTo = w; + updateAttachment(); + } + + void Element::updateAttachment() + { + if (!mRoot) + return; + if (mAttachedTo) + { + if (!mLayer.empty()) + Log(Debug::Warning) << "Ignoring element's layer " << mLayer << " because it's attached to a widget"; + if (mRoot->widget()->getParent() != mAttachedTo) + { + mRoot->widget()->attachToWidget(mAttachedTo); + mRoot->updateCoord(); + } + } + else + { + if (mRoot->widget()->getParent() != nullptr) + { + mRoot->widget()->detachFromWidget(); + } + } + } } diff --git a/components/lua_ui/element.hpp b/components/lua_ui/element.hpp index daaf340660..95d0aa2ebc 100644 --- a/components/lua_ui/element.hpp +++ b/components/lua_ui/element.hpp @@ -9,8 +9,10 @@ namespace LuaUi { static std::shared_ptr make(sol::table layout); - LuaUi::WidgetExtension* mRoot; + WidgetExtension* mRoot; + MyGUI::Widget* mAttachedTo; sol::table mLayout; + std::string mLayer; bool mUpdate; bool mDestroy; @@ -22,9 +24,12 @@ namespace LuaUi friend void clearUserInterface(); + void attachToWidget(MyGUI::Widget* w = nullptr); + private: Element(sol::table layout); static std::map> sAllElements; + void updateAttachment(); }; } diff --git a/components/lua_ui/scriptsettings.cpp b/components/lua_ui/scriptsettings.cpp index 071ded1b5d..c8a7d849c9 100644 --- a/components/lua_ui/scriptsettings.cpp +++ b/components/lua_ui/scriptsettings.cpp @@ -26,12 +26,9 @@ namespace LuaUi allSettings.clear(); } - void attachToWidget(const ScriptSettings& script, MyGUI::Widget* widget) + void attachToWidget(size_t index, MyGUI::Widget* widget) { - WidgetExtension* root = script.mElement->mRoot; - if (!root) - return; - root->widget()->attachToWidget(widget); - root->updateCoord(); + if (0 <= index && index < allSettings.size()) + allSettings[index].mElement->attachToWidget(widget); } } diff --git a/components/lua_ui/scriptsettings.hpp b/components/lua_ui/scriptsettings.hpp index eadcec430f..b13bfc578c 100644 --- a/components/lua_ui/scriptsettings.hpp +++ b/components/lua_ui/scriptsettings.hpp @@ -19,7 +19,7 @@ namespace LuaUi const std::vector& scriptSettings(); void registerSettings(const ScriptSettings& script); void clearSettings(); - void attachToWidget(const ScriptSettings& script, MyGUI::Widget* widget); + void attachToWidget(size_t index, MyGUI::Widget* widget = nullptr); } #endif // !OPENMW_LUAUI_SCRIPTSETTINGS