[Libreoffice-commits] core.git: cui/source

[Stephan Bergmann (via logerrit)]

 cui/source/tabpages/tpbitmap.cxx |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

New commits:
commit 4d59436258702251a881a007ccc52ffd5a3eeb38
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Jan 7 18:06:09 2020 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Jan 7 20:28:35 2020 +0100

    Fix SfxPoolItem use-after-free
    
    ...as observed with -fsanitize=address in Draw, after drawing some rectangle (so
    that there is at least one marked object) doing "Format - Area... - Area -
    Bitmap":
    
    > ERROR: AddressSanitizer: heap-use-after-free on address 0x6030004aca50 at pc 0x7f14d0ef5fe1 bp 0x7ffd966c6cb0 sp 0x7ffd966c6ca8
    > READ of size 4 at 0x6030004aca50 thread T0
    >  #0 in CntUInt32Item::GetValue() const at include/svl/cintitem.hxx:163:42
    >  #1 in SvxBitmapTabPage::Reset(SfxItemSet const*) at cui/source/tabpages/tpbitmap.cxx:278:124
    >  #2 in SvxAreaTabPage::CreatePage(int, SfxTabPage*) at cui/source/tabpages/tparea.cxx:448:21
    >  #3 in SvxAreaTabPage::SelectFillType(weld::ToggleButton&, SfxItemSet const*) at cui/source/tabpages/tparea.cxx:381:9
    >  #4 in SvxAreaTabPage::SelectFillTypeHdl_Impl(weld::ToggleButton&) at cui/source/tabpages/tparea.cxx:364:5
    >  #5 in SvxAreaTabPage::LinkStubSelectFillTypeHdl_Impl(void*, weld::ToggleButton&) at cui/source/tabpages/tparea.cxx:358:1
    >  #6 in Link<weld::ToggleButton&, void>::Call(weld::ToggleButton&) const at include/tools/link.hxx:111:45
    >  #7 in weld::ToggleButton::signal_toggled() at include/vcl/weld.hxx:1130:42
    [...]
    > 0x6030004aca50 is located 16 bytes inside of 24-byte region [0x6030004aca40,0x6030004aca58)
    > freed by thread T0 here:
    >  #0 in operator delete(void*, unsigned long) at ~/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:172:3
    >  #1 in SfxUInt32Item::~SfxUInt32Item() at include/svl/intitem.hxx:113:21
    >  #2 in SfxItemPool::Remove(SfxPoolItem const&) at svl/source/items/itempool.cxx:710:13
    >  #3 in SfxItemSet::~SfxItemSet() at svl/source/items/itemset.cxx:252:42
    >  #4 in SvxBitmapTabPage::Reset(SfxItemSet const*) at cui/source/tabpages/tpbitmap.cxx:276:9
    >  #5 in SvxAreaTabPage::CreatePage(int, SfxTabPage*) at cui/source/tabpages/tparea.cxx:448:21
    >  #6 in SvxAreaTabPage::SelectFillType(weld::ToggleButton&, SfxItemSet const*) at cui/source/tabpages/tparea.cxx:381:9
    >  #7 in SvxAreaTabPage::SelectFillTypeHdl_Impl(weld::ToggleButton&) at cui/source/tabpages/tparea.cxx:364:5
    >  #8 in SvxAreaTabPage::LinkStubSelectFillTypeHdl_Impl(void*, weld::ToggleButton&) at cui/source/tabpages/tparea.cxx:358:1
    >  #9 in Link<weld::ToggleButton&, void>::Call(weld::ToggleButton&) const at include/tools/link.hxx:111:45
    
    This appears to be broken ever since d543d66a4ee34d3b0088f45951b56c150f7206ec
    "tdf#104615: there's no mpView when opening odc directly".
    
    Change-Id: Id0b3991f3e953ca5b10f466daab890383b0428ca
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86368
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/cui/source/tabpages/tpbitmap.cxx b/cui/source/tabpages/tpbitmap.cxx
index 945d92315c94..b18cfffbe607 100644
--- a/cui/source/tabpages/tpbitmap.cxx
+++ b/cui/source/tabpages/tpbitmap.cxx
@@ -260,8 +260,8 @@ bool SvxBitmapTabPage::FillItemSet( SfxItemSet* rAttrs )
 
 void SvxBitmapTabPage::Reset( const SfxItemSet* rAttrs )
 {
-    const SfxPoolItem* pItemTransfWidth = nullptr;
-    const SfxPoolItem* pItemTransfHeight = nullptr;
+    double transfWidth = 0.0;
+    double transfHeight = 0.0;
     double fUIScale  = 1.0;
     if (mpView)
     {
@@ -271,12 +271,12 @@ void SvxBitmapTabPage::Reset( const SfxItemSet* rAttrs )
         if (mpView->AreObjectsMarked())
         {
             SfxItemSet rGeoAttr(mpView->GetGeoAttrFromMarked());
-            pItemTransfWidth = GetItem( rGeoAttr, SID_ATTR_TRANSFORM_WIDTH );
-            pItemTransfHeight= GetItem( rGeoAttr, SID_ATTR_TRANSFORM_HEIGHT );
+            transfWidth = static_cast<double>(GetItem( rGeoAttr, SID_ATTR_TRANSFORM_WIDTH )->GetValue());
+            transfHeight= static_cast<double>(GetItem( rGeoAttr, SID_ATTR_TRANSFORM_HEIGHT )->GetValue());
         }
     }
-    m_fObjectWidth = std::max( pItemTransfWidth ? static_cast<double>(static_cast<const SfxUInt32Item*>(pItemTransfWidth)->GetValue()) : 0.0, 1.0 );
-    m_fObjectHeight = std::max( pItemTransfHeight ? static_cast<double>(static_cast<const SfxUInt32Item*>(pItemTransfHeight)->GetValue()) : 0.0, 1.0 );
+    m_fObjectWidth = std::max( transfWidth, 1.0 );
+    m_fObjectHeight = std::max( transfHeight, 1.0 );
     double fTmpWidth((OutputDevice::LogicToLogic(static_cast<sal_Int32>(m_fObjectWidth), mePoolUnit, MapUnit::Map100thMM )) / fUIScale);
     m_fObjectWidth = fTmpWidth;