From 8e7c01b5615f295611530e7cda61c5b10083370a Mon Sep 17 00:00:00 2001 From: Ilya Zhuravlev Date: Mon, 24 Dec 2018 01:50:58 -0500 Subject: [PATCH] loadingscreen: Fix UaF in loading screen. When the CopyFramebufferToTextureCallback callback is called, in its operator() it resets setInitialDrawCallback by providing a NULL pointer. However, this causes the callback to get deleted. In turn, the "this" pointer is invalidated. When execution returns to DrawCallback::run, it accesses a _nestedCallback member of deleted "this" which is UB. --- apps/openmw/mwgui/loadingscreen.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/openmw/mwgui/loadingscreen.cpp b/apps/openmw/mwgui/loadingscreen.cpp index 7f641d408..68cf0439a 100644 --- a/apps/openmw/mwgui/loadingscreen.cpp +++ b/apps/openmw/mwgui/loadingscreen.cpp @@ -141,10 +141,6 @@ namespace MWGui int w = renderInfo.getCurrentCamera()->getViewport()->width(); int h = renderInfo.getCurrentCamera()->getViewport()->height(); mTexture->copyTexImage2D(*renderInfo.getState(), 0, 0, w, h); - - // Callback removes itself when done - if (renderInfo.getCurrentCamera()) - renderInfo.getCurrentCamera()->setInitialDrawCallback(nullptr); } private: @@ -308,6 +304,8 @@ namespace MWGui mGuiTexture.reset(new osgMyGUI::OSGTexture(mTexture)); } + // Notice that the next time this is called, the current CopyFramebufferToTextureCallback will be deleted + // so there's no memory leak as at most one object of type CopyFramebufferToTextureCallback is allocated at a time. mViewer->getCamera()->setInitialDrawCallback(new CopyFramebufferToTextureCallback(mTexture)); mBackgroundImage->setBackgroundImage("");