You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openoffice.apache.org by js...@apache.org on 2011/12/01 11:23:40 UTC
svn commit: r1209022 - in /incubator/ooo/trunk/main/filter/source/msfilter:
dffrecordheader.cxx msdffimp.cxx
Author: jsc
Date: Thu Dec 1 10:23:35 2011
New Revision: 1209022
URL: http://svn.apache.org/viewvc?rev=1209022&view=rev
Log:
add some additional checks to ensure proper reading operations
Modified:
incubator/ooo/trunk/main/filter/source/msfilter/dffrecordheader.cxx
incubator/ooo/trunk/main/filter/source/msfilter/msdffimp.cxx
Modified: incubator/ooo/trunk/main/filter/source/msfilter/dffrecordheader.cxx
URL: http://svn.apache.org/viewvc/incubator/ooo/trunk/main/filter/source/msfilter/dffrecordheader.cxx?rev=1209022&r1=1209021&r2=1209022&view=diff
==============================================================================
--- incubator/ooo/trunk/main/filter/source/msfilter/dffrecordheader.cxx (original)
+++ incubator/ooo/trunk/main/filter/source/msfilter/dffrecordheader.cxx Thu Dec 1 10:23:35 2011
@@ -35,6 +35,8 @@ SvStream& operator>>( SvStream& rIn, Dff
rRec.nRecInstance = nTmp >> 4;
rIn >> rRec.nRecType;
rIn >> rRec.nRecLen;
+ if ( rRec.nRecLen > ( SAL_MAX_UINT32 - rRec.nFilePos ) ) // preserving overflow, optimal would be to check
+ rIn.SetError( SVSTREAM_FILEFORMAT_ERROR ); // the record size against the parent header
return rIn;
}
Modified: incubator/ooo/trunk/main/filter/source/msfilter/msdffimp.cxx
URL: http://svn.apache.org/viewvc/incubator/ooo/trunk/main/filter/source/msfilter/msdffimp.cxx?rev=1209022&r1=1209021&r2=1209022&view=diff
==============================================================================
--- incubator/ooo/trunk/main/filter/source/msfilter/msdffimp.cxx (original)
+++ incubator/ooo/trunk/main/filter/source/msfilter/msdffimp.cxx Thu Dec 1 10:23:35 2011
@@ -3225,7 +3225,7 @@ sal_Bool SvxMSDffManager::SeekToShape( S
rSt >> aEscherF002Hd;
sal_uLong nEscherF002End = aEscherF002Hd.GetRecEndFilePos();
DffRecordHeader aEscherObjListHd;
- while ( rSt.Tell() < nEscherF002End )
+ while ( ( rSt.GetError() == 0 ) && ( rSt.Tell() < nEscherF002End ) )
{
rSt >> aEscherObjListHd;
if ( aEscherObjListHd.nRecVer != 0xf )
@@ -3257,11 +3257,20 @@ sal_Bool SvxMSDffManager::SeekToShape( S
FASTBOOL SvxMSDffManager::SeekToRec( SvStream& rSt, sal_uInt16 nRecId, sal_uLong nMaxFilePos, DffRecordHeader* pRecHd, sal_uLong nSkipCount ) const
{
FASTBOOL bRet = sal_False;
- sal_uLong nFPosMerk = rSt.Tell(); // FilePos merken fuer ggf. spaetere Restauration
+ sal_uLong nFPosMerk = rSt.Tell(); // store FilePos to restore it later if necessary
DffRecordHeader aHd;
do
{
rSt >> aHd;
+
+ // check potential error reading and if seeking to the end of record is possible at all.
+ // It is probably cheaper instead of doing the file seek operation
+ if ( rSt.GetError() || ( aHd.GetRecEndFilePos() > nMaxFilePos ) )
+ {
+ bRet= sal_False;
+ break;
+ }
+
if ( aHd.nRecType == nRecId )
{
if ( nSkipCount )
@@ -3280,7 +3289,7 @@ FASTBOOL SvxMSDffManager::SeekToRec( SvS
}
while ( rSt.GetError() == 0 && rSt.Tell() < nMaxFilePos && !bRet );
if ( !bRet )
- rSt.Seek( nFPosMerk ); // FilePos restaurieren
+ rSt.Seek( nFPosMerk ); // restore orginal FilePos
return bRet;
}
@@ -5678,12 +5687,15 @@ void SvxMSDffManager::GetFidclData( long
{
if ( aDggAtomHd.nRecLen == ( mnIdClusters * sizeof( FIDCL ) + 16 ) )
{
- mpFidcls = new FIDCL[ mnIdClusters ];
- for ( sal_uInt32 i = 0; i < mnIdClusters; i++ )
- {
- rStCtrl >> mpFidcls[ i ].dgid
- >> mpFidcls[ i ].cspidCur;
- }
+ //mpFidcls = new FIDCL[ mnIdClusters ];
+ mpFidcls = new (std::nothrow) FIDCL[ mnIdClusters ];
+ if ( mpFidcls ) {
+ for ( sal_uInt32 i = 0; i < mnIdClusters; i++ )
+ {
+ rStCtrl >> mpFidcls[ i ].dgid
+ >> mpFidcls[ i ].cspidCur;
+ }
+ }
}
}
}
@@ -5809,7 +5821,7 @@ void SvxMSDffManager::GetCtrlData( long
if( !bOk )
{
- nPos++;
+ nPos++; // ????????? TODO: trying to get an one-hit wonder, this code code should be rewritten...
rStCtrl.Seek( nPos );
bOk = ReadCommonRecordHeader( rStCtrl, nVer, nInst, nFbt, nLength )
&& ( DFF_msofbtDgContainer == nFbt );
@@ -5825,7 +5837,7 @@ void SvxMSDffManager::GetCtrlData( long
++nDrawingContainerId;
// <--
}
- while( nPos < nMaxStrPos && bOk );
+ while( ( rStCtrl.GetError() == 0 ) && ( nPos < nMaxStrPos ) && bOk );
}
}
@@ -6553,6 +6565,8 @@ sal_Bool SvxMSDffManager::ReadCommonReco
rSt >> nTmp >> rFbt >> rLength;
rVer = sal::static_int_cast< sal_uInt8 >(nTmp & 15);
rInst = nTmp >> 4;
+ if ( rLength > ( SAL_MAX_UINT32 - rSt.Tell() ) ) // preserving overflow, optimal would be to check
+ rSt.SetError( SVSTREAM_FILEFORMAT_ERROR ); // the record size against the parent header
return rSt.GetError() == 0;
}
@@ -6564,7 +6578,7 @@ sal_Bool SvxMSDffManager::ProcessClientA
{
if( nDatLen )
{
- rpBuff = new char[ nDatLen ];
+ rpBuff = new (std::nothrow) char[ nDatLen ];
rBuffLen = nDatLen;
rStData.Read( rpBuff, nDatLen );
}
@@ -6576,9 +6590,12 @@ sal_Bool SvxMSDffManager::ProcessClientD
{
if( nDatLen )
{
- rpBuff = new char[ nDatLen ];
- rBuffLen = nDatLen;
- rStData.Read( rpBuff, nDatLen );
+ rpBuff = new (std::nothrow) char[ nDatLen ];
+ if ( rpBuff )
+ {
+ rBuffLen = nDatLen;
+ rStData.Read( rpBuff, nDatLen );
+ }
}
return sal_True;
}