sc/source/filter/excel/tokstack.cxx | 79 +++++++++++++++++++----------------- sc/source/filter/inc/tokstack.hxx | 24 ++++++++++ sc/source/filter/lotus/lotform.cxx | 27 +++++++++--- 3 files changed, 85 insertions(+), 45 deletions(-)
New commits: commit 6e2f927704980c0057d5deb3a6bc3ef5540379a4 Author: Eike Rathke <er...@redhat.com> Date: Fri Dec 1 10:54:01 2017 +0100 ofz: guard against binary crap argument counts and ID/OpCode generation This is a combination of 2 commits. ofz: guard against binary crap argument counts and ID/OpCode generation (cherry picked from commit 889c72a7e54f241342f42b1b0a05858902228cbc) Do not force non-existent parameters into the TokenPool, ofz-related (cherry picked from commit e6ced1496da9580cf885cce1a2fc9f67528c3a0e) 2fa0ae81b987af592c14486040077c9ff157fab9 Change-Id: I60e181729713f3b202e880707a79e9da80d9d85d Reviewed-on: https://gerrit.libreoffice.org/49161 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Caolán McNamara <caol...@redhat.com> Tested-by: Caolán McNamara <caol...@redhat.com> diff --git a/sc/source/filter/excel/tokstack.cxx b/sc/source/filter/excel/tokstack.cxx index 39a33dddd4d5..4129b50f2287 100644 --- a/sc/source/filter/excel/tokstack.cxx +++ b/sc/source/filter/excel/tokstack.cxx @@ -108,6 +108,21 @@ bool TokenPool::GrowId() return true; } +bool TokenPool::CheckElementOrGrow() +{ + // Last possible ID to be assigned somewhere is nElementAkt+1 + if (nElementAkt + 1 == nScTokenOff - 1) + { + SAL_WARN("sc.filter","TokenPool::CheckElementOrGrow - last possible ID " << nElementAkt+1); + return false; + } + + if (nElementAkt >= nElement) + return GrowElement(); + + return true; +} + bool TokenPool::GrowElement() { sal_uInt16 nElementNew = lcl_canGrow( nElement); @@ -161,9 +176,11 @@ bool TokenPool::GrowMatrix() bool TokenPool::GetElement( const sal_uInt16 nId ) { - OSL_ENSURE( nId < nElementAkt, "*TokenPool::GetElement(): Id too large!?" ); if (nId >= nElementAkt) + { + SAL_WARN("sc.filter","TokenPool::GetElement - Id too large, " << nId << " >= " << nElementAkt); return false; + } bool bRet = true; if( pType[ nId ] == T_Id ) @@ -394,9 +411,8 @@ void TokenPool::operator >>( TokenId& rId ) { rId = static_cast<TokenId>( nElementAkt + 1 ); - if( nElementAkt >= nElement ) - if (!GrowElement()) - return; + if (!CheckElementOrGrow()) + return; pElement[ nElementAkt ] = nP_IdLast; // Start of Token-sequence pType[ nElementAkt ] = T_Id; // set Typeinfo @@ -409,9 +425,8 @@ void TokenPool::operator >>( TokenId& rId ) const TokenId TokenPool::Store( const double& rDouble ) { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( pP_Dbl.m_writemark >= pP_Dbl.m_capacity ) if (!pP_Dbl.Grow()) @@ -438,9 +453,8 @@ const TokenId TokenPool::Store( const sal_uInt16 nIndex ) const TokenId TokenPool::Store( const OUString& rString ) { // mostly copied to Store( const sal_Char* ), to avoid a temporary string - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( ppP_Str.m_writemark >= ppP_Str.m_capacity ) if (!ppP_Str.Grow()) @@ -468,9 +482,8 @@ const TokenId TokenPool::Store( const OUString& rString ) const TokenId TokenPool::Store( const ScSingleRefData& rTr ) { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( ppP_RefTr.m_writemark >= ppP_RefTr.m_capacity ) if (!ppP_RefTr.Grow()) @@ -492,9 +505,8 @@ const TokenId TokenPool::Store( const ScSingleRefData& rTr ) const TokenId TokenPool::Store( const ScComplexRefData& rTr ) { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( ppP_RefTr.m_writemark + 1 >= ppP_RefTr.m_capacity ) if (!ppP_RefTr.Grow(2)) @@ -522,9 +534,8 @@ const TokenId TokenPool::Store( const ScComplexRefData& rTr ) const TokenId TokenPool::Store( const DefTokenId e, const OUString& r ) { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( ppP_Ext.m_writemark >= ppP_Ext.m_capacity ) if (!ppP_Ext.Grow()) @@ -549,9 +560,8 @@ const TokenId TokenPool::Store( const DefTokenId e, const OUString& r ) const TokenId TokenPool::StoreNlf( const ScSingleRefData& rTr ) { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( ppP_Nlf.m_writemark >= ppP_Nlf.m_capacity ) if (!ppP_Nlf.Grow()) @@ -575,9 +585,8 @@ const TokenId TokenPool::StoreNlf( const ScSingleRefData& rTr ) const TokenId TokenPool::StoreMatrix() { - if( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); if( nP_MatrixAkt >= nP_Matrix ) if (!GrowMatrix()) @@ -598,9 +607,8 @@ const TokenId TokenPool::StoreMatrix() const TokenId TokenPool::StoreName( sal_uInt16 nIndex, sal_Int16 nSheet ) { - if ( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); pElement[nElementAkt] = static_cast<sal_uInt16>(maRangeNames.size()); pType[nElementAkt] = T_RN; @@ -617,9 +625,8 @@ const TokenId TokenPool::StoreName( sal_uInt16 nIndex, sal_Int16 nSheet ) const TokenId TokenPool::StoreExtName( sal_uInt16 nFileId, const OUString& rName ) { - if ( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); pElement[nElementAkt] = static_cast<sal_uInt16>(maExtNames.size()); pType[nElementAkt] = T_ExtName; @@ -636,9 +643,8 @@ const TokenId TokenPool::StoreExtName( sal_uInt16 nFileId, const OUString& rName const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabName, const ScSingleRefData& rRef ) { - if ( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); pElement[nElementAkt] = static_cast<sal_uInt16>(maExtCellRefs.size()); pType[nElementAkt] = T_ExtRefC; @@ -656,9 +662,8 @@ const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabNa const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabName, const ScComplexRefData& rRef ) { - if ( nElementAkt >= nElement ) - if (!GrowElement()) - return static_cast<const TokenId>(nElementAkt+1); + if (!CheckElementOrGrow()) + return static_cast<const TokenId>(nElementAkt+1); pElement[nElementAkt] = static_cast<sal_uInt16>(maExtAreaRefs.size()); pType[nElementAkt] = T_ExtRefA; diff --git a/sc/source/filter/inc/tokstack.hxx b/sc/source/filter/inc/tokstack.hxx index 1e7e0eeb1582..eef80f072c26 100644 --- a/sc/source/filter/inc/tokstack.hxx +++ b/sc/source/filter/inc/tokstack.hxx @@ -217,6 +217,13 @@ private: bool GrowId(); bool GrowElement(); bool GrowMatrix(); + /** @return false means nElementAkt range + below nScTokenOff would overflow or + further allocation is not possible, no + new ID available other than + nElementAkt+1. + */ + bool CheckElementOrGrow(); bool GetElement( const sal_uInt16 nId ); bool GetElementRek( const sal_uInt16 nId ); void ClearMatrix(); @@ -317,6 +324,7 @@ inline void TokenStack::operator >>( TokenId& rId ) else { SAL_WARN("sc.filter", "*TokenStack::>>(): is empty, is empty, ..."); + rId = 0; } } @@ -331,7 +339,13 @@ inline TokenPool& TokenPool::operator <<( const TokenId& rId ) // finalize with >> or Store() // rId -> ( sal_uInt16 ) rId - 1; sal_uInt16 nId = static_cast<sal_uInt16>(rId); - if (nId >= nScTokenOff) + if (nId == 0) + { + // This would result in nId-1==0xffff, create error. + SAL_WARN("sc.filter", "-TokenPool::operator <<: TokenId 0"); + nId = static_cast<sal_uInt16>(ocErrNull) + nScTokenOff + 1; + } + else if (nId >= nScTokenOff) { SAL_WARN("sc.filter", "-TokenPool::operator <<: TokenId in DefToken-Range! " << static_cast<sal_uInt16>(rId)); @@ -374,7 +388,13 @@ inline TokenPool& TokenPool::operator <<( TokenStack& rStack ) if (!GrowId()) return *this; - pP_Id[ nP_IdAkt ] = ( ( sal_uInt16 ) rStack.Get() ) - 1; + sal_uInt16 nId = static_cast<sal_uInt16>(rStack.Get()); + if (nId == 0) + { + // Indicates error, so generate one. Empty stack, overflow, ... + nId = static_cast<sal_uInt16>(ocErrNull) + nScTokenOff + 1; + } + pP_Id[ nP_IdAkt ] = nId - 1; nP_IdAkt++; return *this; diff --git a/sc/source/filter/lotus/lotform.cxx b/sc/source/filter/lotus/lotform.cxx index 068a77f5bb46..091da56e9dbb 100644 --- a/sc/source/filter/lotus/lotform.cxx +++ b/sc/source/filter/lotus/lotform.cxx @@ -77,9 +77,13 @@ void LotusToSc::DoFunc( DefTokenId eOc, sal_uInt8 nCnt, const sal_Char* pExtStri } } - for( nLauf = 0 ; nLauf < nCnt ; nLauf++ ) + for( nLauf = 0 ; nLauf < nCnt && aStack.HasMoreTokens() ; nLauf++ ) aStack >> eParam[ nLauf ]; + if (nLauf < nCnt) + // Adapt count to reality. All sort of binary crap is possible. + nCnt = static_cast<sal_uInt8>(nLauf); + // special cases... switch( eOc ) { @@ -189,12 +193,23 @@ void LotusToSc::DoFunc( DefTokenId eOc, sal_uInt8 nCnt, const sal_Char* pExtStri sal_Int16 nLast = nCnt - 1; if( eOc == ocPMT ) - { // special case ocPMT, ignore (negate?) last parameter! + { // special case ocPMT, negate last parameter! // additionally: 1. -> 3., 3. -> 2., 2. -> 1. - SAL_WARN_IF( nCnt != 3, "sc", - "+LotusToSc::DoFunc(): ocPMT needs 3 parameters!" ); - aPool << eParam[ 1 ] << ocSep << eParam[ 0 ] << ocSep - << ocNegSub << eParam[ 2 ]; + SAL_WARN_IF( nCnt != 3, "sc", "+LotusToSc::DoFunc(): ocPMT needs 3 parameters!" ); + // There should be at least 3 arguments, but with binary crap may not.. + switch (nCnt) + { + case 1: + aPool << eParam[ 1 ]; + break; + case 2: + aPool << eParam[ 1 ] << ocSep << eParam[ 0 ]; + break; + default: + case 3: + aPool << eParam[ 1 ] << ocSep << eParam[ 0 ] << ocSep << ocNegSub << eParam[ 2 ]; + break; + } } else { // default _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits