From 18d56818f56d567ce66f3ffe86a11c4a7fb458e0 Mon Sep 17 00:00:00 2001 From: Ilya Sinitsyn Date: Mon, 12 Oct 2020 22:37:17 +0700 Subject: [PATCH] Fix possible use of dangling window pointer in wxSizer::Insert() Add the sizer item to the sizers items list only after calling of wxWindowBase::SetContainingSizer() because it can throw (if it asserts and the assert handler throws an exception, as happens in our own unit tests) and then the sizer item would be kept in the sizers items list but m_containingSizer wouldn't be set for the window. --- include/wx/sizer.h | 4 ++++ src/common/sizer.cpp | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/include/wx/sizer.h b/include/wx/sizer.h index 4a51b11c3f..dd15744a64 100644 --- a/include/wx/sizer.h +++ b/include/wx/sizer.h @@ -321,6 +321,10 @@ public: // Enable deleting the SizerItem without destroying the contained sizer. void DetachSizer() { m_sizer = NULL; } + // Enable deleting the SizerItem without resetting the sizer in the + // contained window. + void DetachWindow() { m_window = NULL; m_kind = Item_None; } + virtual wxSize GetSize() const; virtual wxSize CalcMin(); virtual void SetDimension( const wxPoint& pos, const wxSize& size ); diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index de3fc956e8..0a96b13eed 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -673,7 +673,35 @@ wxSizer::~wxSizer() wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item ) { - m_children.Insert( index, item ); + // The helper class that solves two problems when + // wxWindowBase::SetContainingSizer() throws: + // 1. Avoid leaking memory using the scoped pointer to the sizer item. + // 2. Disassociate the window from the sizer item to not reset the + // containing sizer for the window in the items destructor. + class ContainingSizerGuard + { + public: + explicit ContainingSizerGuard( wxSizerItem *item ) + : m_item(item) + { + } + + ~ContainingSizerGuard() + { + if ( m_item ) + m_item->DetachWindow(); + } + + wxSizerItem* Release() + { + return m_item.release(); + } + + private: + wxScopedPtr m_item; + }; + + ContainingSizerGuard guard( item ); if ( item->GetWindow() ) item->GetWindow()->SetContainingSizer( this ); @@ -681,7 +709,9 @@ wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item ) if ( item->GetSizer() ) item->GetSizer()->SetContainingWindow( m_containingWindow ); - return item; + m_children.Insert( index, item ); + + return guard.Release(); } void wxSizer::SetContainingWindow(wxWindow *win)