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

Caolán McNamara (via logerrit) logerrit at kemper.freedesktop.org
Mon May 31 09:47:15 UTC 2021


 sc/source/filter/inc/defnamesbuffer.hxx |    2 +-
 sc/source/filter/inc/workbookhelper.hxx |    7 +++++--
 sc/source/filter/oox/defnamesbuffer.cxx |   23 ++++++++++++-----------
 sc/source/filter/oox/workbookhelper.cxx |   32 +++++++++++++++++---------------
 4 files changed, 35 insertions(+), 29 deletions(-)

New commits:
commit 9bf1089965672d3825b587cbd888093ac362013e
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Mon May 31 09:30:52 2021 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Mon May 31 11:46:32 2021 +0200

    crashtesting: use after free on export of tdf122510-1.xlsx to ods
    
    and also tdf95549-3.xlsm
    
    related to:
    
    commit f8c1048eb437b1e685b76198165844e2ecc97a56
    Date:   Wed May 19 19:04:02 2021 +0200
    
        fix leak in oox import
    
    and:
    
    commit 3cd6402c5443c8069c07d9e420d5ef5b43af6bef
    Date:   Thu May 6 18:47:30 2021 +0200
    
        tdf#127301 XLSX import: hide hidden named range of autofilter
    
    clearly this is fragile so just explicitly return who owns the
    ScRangeData*
    
    Change-Id: Ic3210bb8788bbbc85609bb384fa4a4625c15e487
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/116432
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sc/source/filter/inc/defnamesbuffer.hxx b/sc/source/filter/inc/defnamesbuffer.hxx
index 03eec97af201..3200bd5dc8de 100644
--- a/sc/source/filter/inc/defnamesbuffer.hxx
+++ b/sc/source/filter/inc/defnamesbuffer.hxx
@@ -122,7 +122,7 @@ public:
 private:
     typedef ::std::unique_ptr< StreamDataSequence >   StreamDataSeqPtr;
 
-    ScRangeData*        mpScRangeData;       /// ScRangeData of the defined name.
+    RangeDataRet        maScRangeData;      /// ScRangeData of the defined name.
     sal_Int32           mnTokenIndex;       /// Name index used in API token array.
     sal_Int16           mnCalcSheet;        /// Calc sheet index for sheet-local names.
     sal_Unicode         mcBuiltinId;        /// Identifier for built-in defined names.
diff --git a/sc/source/filter/inc/workbookhelper.hxx b/sc/source/filter/inc/workbookhelper.hxx
index e6633d15f4f9..d6d04007d39c 100644
--- a/sc/source/filter/inc/workbookhelper.hxx
+++ b/sc/source/filter/inc/workbookhelper.hxx
@@ -160,10 +160,13 @@ public:
     css::uno::Reference< css::style::XStyle >
                         getStyleObject( const OUString& rStyleName, bool bPageStyle ) const;
 
+    // second is true if ownership belongs to the caller
+    typedef std::pair<ScRangeData*, bool> RangeDataRet;
+
     /** Creates and returns a defined name on-the-fly in the Calc document.
         The name will not be buffered in the global defined names buffer.
         @param orName  (in/out-parameter) Returns the resulting used name. */
-    ScRangeData* createNamedRangeObject(
+    RangeDataRet createNamedRangeObject(
                             OUString& orName,
                             const css::uno::Sequence< css::sheet::FormulaToken>& rTokens,
                             sal_Int32 nIndex,
@@ -172,7 +175,7 @@ public:
     /** Creates and returns a defined name on-the-fly in the sheet.
         The name will not be buffered in the global defined names buffer.
         @param orName  (in/out-parameter) Returns the resulting used name. */
-    ScRangeData* createLocalNamedRangeObject(
+    RangeDataRet createLocalNamedRangeObject(
                             OUString& orName,
                             const css::uno::Sequence< css::sheet::FormulaToken>& rTokens,
                             sal_Int32 nIndex,
diff --git a/sc/source/filter/oox/defnamesbuffer.cxx b/sc/source/filter/oox/defnamesbuffer.cxx
index a554ab9cd0f1..9eebaf1633a7 100644
--- a/sc/source/filter/oox/defnamesbuffer.cxx
+++ b/sc/source/filter/oox/defnamesbuffer.cxx
@@ -146,7 +146,7 @@ const OUString& DefinedNameBase::getUpcaseModelName() const
 
 DefinedName::DefinedName( const WorkbookHelper& rHelper ) :
     DefinedNameBase( rHelper ),
-    mpScRangeData(nullptr),
+    maScRangeData(nullptr, false),
     mnTokenIndex( -1 ),
     mnCalcSheet( 0 ),
     mcBuiltinId( BIFF_DEFNAME_UNKNOWN )
@@ -233,9 +233,9 @@ void DefinedName::createNameObject( sal_Int32 nIndex )
 
     // create the name and insert it into the document, maCalcName will be changed to the resulting name
     if (maModel.mnSheet >= 0)
-        mpScRangeData = createLocalNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mnSheet, maModel.mbHidden );
+        maScRangeData = createLocalNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mnSheet, maModel.mbHidden );
     else
-        mpScRangeData = createNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mbHidden );
+        maScRangeData = createNamedRangeObject( maCalcName, ApiTokenSequence(), nIndex, nNameFlags, maModel.mbHidden );
     mnTokenIndex = nIndex;
 }
 
@@ -259,17 +259,18 @@ std::unique_ptr<ScTokenArray> DefinedName::getScTokens(
 
 void DefinedName::convertFormula( const css::uno::Sequence<css::sheet::ExternalLinkInfo>& rExternalLinks )
 {
+    ScRangeData* pScRangeData = maScRangeData.first;
     // macro function or vba procedure
-    if(!mpScRangeData)
+    if (!pScRangeData)
         return;
 
     // convert and set formula of the defined name
     {
         std::unique_ptr<ScTokenArray> pTokenArray = getScTokens( rExternalLinks);
-        mpScRangeData->SetCode( *pTokenArray );
+        pScRangeData->SetCode( *pTokenArray );
     }
 
-    ScTokenArray* pTokenArray = mpScRangeData->GetCode();
+    ScTokenArray* pTokenArray = pScRangeData->GetCode();
     Sequence< FormulaToken > aFTokenSeq;
     ScTokenConversion::ConvertToTokenSequence( getScDocument(), aFTokenSeq, *pTokenArray );
     // set built-in names (print ranges, repeated titles, filter ranges)
@@ -327,7 +328,8 @@ void DefinedName::convertFormula( const css::uno::Sequence<css::sheet::ExternalL
 
 bool DefinedName::getAbsoluteRange( ScRange& orRange ) const
 {
-    ScTokenArray* pTokenArray = mpScRangeData->GetCode();
+    ScRangeData* pScRangeData = maScRangeData.first;
+    ScTokenArray* pTokenArray = pScRangeData->GetCode();
     Sequence< FormulaToken > aFTokenSeq;
     ScTokenConversion::ConvertToTokenSequence(getScDocument(), aFTokenSeq, *pTokenArray);
     return getFormulaParser().extractCellRange( orRange, aFTokenSeq );
@@ -336,12 +338,11 @@ bool DefinedName::getAbsoluteRange( ScRange& orRange ) const
 DefinedName::~DefinedName()
 {
     // this kind of field is owned by us - see lcl_addNewByNameAndTokens
-    if (mpScRangeData && maModel.mbHidden &&
-        (mcBuiltinId == BIFF_DEFNAME_CRITERIA || mcBuiltinId == BIFF_DEFNAME_FILTERDATABASE))
-        delete mpScRangeData;
+    bool bOwned = maScRangeData.second;
+    if (bOwned)
+        delete maScRangeData.first;
 }
 
-
 DefinedNamesBuffer::DefinedNamesBuffer( const WorkbookHelper& rHelper ) :
     WorkbookHelper( rHelper )
 {
diff --git a/sc/source/filter/oox/workbookhelper.cxx b/sc/source/filter/oox/workbookhelper.cxx
index 8c7ffb3de4d8..8a7804102246 100644
--- a/sc/source/filter/oox/workbookhelper.cxx
+++ b/sc/source/filter/oox/workbookhelper.cxx
@@ -149,9 +149,9 @@ public:
     /** Returns the specified cell or page style from the Calc document. */
     Reference< XStyle > getStyleObject( const OUString& rStyleName, bool bPageStyle ) const;
     /** Creates and returns a defined name on-the-fly in the Calc document. */
-    ScRangeData* createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden );
+    WorkbookHelper::RangeDataRet createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden );
     /** Creates and returns a defined name on the-fly in the correct Calc sheet. */
-    ScRangeData* createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden );
+    WorkbookHelper::RangeDataRet createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden );
     /** Creates and returns a database range on-the-fly in the Calc document. */
     Reference< XDatabaseRange > createDatabaseRangeObject( OUString& orName, const ScRange& rRangeAddr );
     /** Creates and returns an unnamed database range on-the-fly in the Calc document. */
@@ -350,7 +350,7 @@ Reference< XStyle > WorkbookGlobals::getStyleObject( const OUString& rStyleName,
 
 namespace {
 
-ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& rName, const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType, bool bHidden )
+WorkbookHelper::RangeDataRet lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& rName, const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType, bool bHidden )
 {
     bool bDone = false;
     ScRangeData::Type nNewType = ScRangeData::Type::Name;
@@ -366,7 +366,9 @@ ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, c
         pNew->SetIndex( nIndex );
     // create but not insert hidden FILTER_CRITERIA named ranges to ScRangeName
     if ( bHidden && nNewType == ScRangeData::Type::Criteria )
-        return pNew;
+    {
+        return WorkbookHelper::RangeDataRet(pNew, true);
+    }
     if ( pNames->insert(pNew) )
         bDone = true;
     if (!bDone)
@@ -374,7 +376,7 @@ ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, c
         delete pNew;
         throw RuntimeException();
     }
-    return pNew;
+    return WorkbookHelper::RangeDataRet(pNew, false);
 }
 
 OUString findUnusedName( const ScRangeName* pRangeName, const OUString& rSuggestedName )
@@ -389,11 +391,11 @@ OUString findUnusedName( const ScRangeName* pRangeName, const OUString& rSuggest
 
 }
 
-ScRangeData* WorkbookGlobals::createNamedRangeObject(
+WorkbookHelper::RangeDataRet WorkbookGlobals::createNamedRangeObject(
     OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden )
 {
     // create the name and insert it into the Calc document
-    ScRangeData* pScRangeData = nullptr;
+    WorkbookHelper::RangeDataRet aScRangeData(nullptr, false);
     if( !orName.isEmpty() )
     {
         ScDocument& rDoc =  getScDocument();
@@ -401,16 +403,16 @@ ScRangeData* WorkbookGlobals::createNamedRangeObject(
         // find an unused name
         orName = findUnusedName( pNames, orName );
         // create the named range
-        pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden );
+        aScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden );
     }
-    return pScRangeData;
+    return aScRangeData;
 }
 
-ScRangeData* WorkbookGlobals::createLocalNamedRangeObject(
+WorkbookHelper::RangeDataRet WorkbookGlobals::createLocalNamedRangeObject(
     OUString& orName, const Sequence< FormulaToken >&  rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden )
 {
     // create the name and insert it into the Calc document
-    ScRangeData* pScRangeData = nullptr;
+    WorkbookHelper::RangeDataRet aScRangeData(nullptr, false);
     if( !orName.isEmpty() )
     {
         ScDocument& rDoc =  getScDocument();
@@ -420,9 +422,9 @@ ScRangeData* WorkbookGlobals::createLocalNamedRangeObject(
         // find an unused name
         orName = findUnusedName( pNames, orName );
         // create the named range
-        pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden );
+        aScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags, bHidden );
     }
-    return pScRangeData;
+    return aScRangeData;
 }
 
 Reference< XDatabaseRange > WorkbookGlobals::createDatabaseRangeObject( OUString& orName, const ScRange& rRangeAddr )
@@ -865,12 +867,12 @@ Reference< XStyle > WorkbookHelper::getStyleObject( const OUString& rStyleName,
     return mrBookGlob.getStyleObject( rStyleName, bPageStyle );
 }
 
-ScRangeData* WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ) const
+WorkbookHelper::RangeDataRet WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, bool bHidden ) const
 {
     return mrBookGlob.createNamedRangeObject( orName, rTokens, nIndex, nNameFlags, bHidden );
 }
 
-ScRangeData* WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ) const
+WorkbookHelper::RangeDataRet WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab, bool bHidden ) const
 {
     return mrBookGlob.createLocalNamedRangeObject( orName, rTokens, nIndex, nNameFlags, nTab, bHidden );
 }


More information about the Libreoffice-commits mailing list