From 28886e4e0cf5fc57385840172129a14ce3c764bc Mon Sep 17 00:00:00 2001 From: Vincent Povirk Date: Thu, 6 May 2010 11:36:41 -0500 Subject: [PATCH 2/2] ole32: Fix reads past the end of streams. Reads past the end of a stream are never an error, and we need to limit the reads based on the size of the stream, not the size of the blocks it uses. Also, fix code that assumes reads past the end of streams are errors internally. --- dlls/ole32/storage32.c | 46 +++++++++++++++++++++++++++----- dlls/ole32/tests/storage32.c | 59 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/dlls/ole32/storage32.c b/dlls/ole32/storage32.c index 9ac794e..bc1fb96 100644 --- a/dlls/ole32/storage32.c +++ b/dlls/ole32/storage32.c @@ -3584,6 +3584,9 @@ HRESULT StorageImpl_ReadRawDirEntry(StorageImpl *This, ULONG index, BYTE *buffer buffer, &bytesRead); + if (bytesRead != RAW_DIRENTRY_SIZE) + return STG_E_READFAULT; + return hr; } @@ -3929,6 +3932,11 @@ BlockChainStream* Storage32Impl_SmallBlocksToBigBlocks( offset.u.LowPart += cbRead; } + else + { + resRead = STG_E_READFAULT; + break; + } } while (cbTotalRead.QuadPart < size.QuadPart); HeapFree(GetProcessHeap(),0,buffer); @@ -4027,6 +4035,11 @@ SmallBlockChainStream* Storage32Impl_BigBlocksToSmallBlocks( offset.u.LowPart += cbRead; } + else + { + resRead = STG_E_READFAULT; + break; + } }while(cbTotalRead.QuadPart < size.QuadPart); HeapFree(GetProcessHeap(), 0, buffer); @@ -5810,6 +5823,7 @@ HRESULT BlockChainStream_ReadAt(BlockChainStream* This, ULONG bytesToReadInBuffer; ULONG blockIndex; BYTE* bufferWalker; + ULARGE_INTEGER stream_size; TRACE("(%p)-> %i %p %i %p\n",This, offset.u.LowPart, buffer, size, bytesRead); @@ -5818,13 +5832,17 @@ HRESULT BlockChainStream_ReadAt(BlockChainStream* This, */ blockIndex = BlockChainStream_GetSectorOfOffset(This, blockNoInSequence); - if (blockIndex == BLOCK_END_OF_CHAIN) - return STG_E_DOCFILECORRUPT; /* We failed to find the starting block */ + *bytesRead = 0; + + stream_size = BlockChainStream_GetSize(This); + if (stream_size.QuadPart > offset.QuadPart) + size = min(stream_size.QuadPart - offset.QuadPart, size); + else + return S_OK; /* * Start reading the buffer. */ - *bytesRead = 0; bufferWalker = buffer; while ( (size > 0) && (blockIndex != BLOCK_END_OF_CHAIN) ) @@ -5862,7 +5880,7 @@ HRESULT BlockChainStream_ReadAt(BlockChainStream* This, break; } - return (size == 0) ? S_OK : STG_E_READFAULT; + return S_OK; } /****************************************************************************** @@ -6295,6 +6313,9 @@ static HRESULT SmallBlockChainStream_GetNextBlockInChain( &buffer, &bytesRead); + if (SUCCEEDED(res) && bytesRead != sizeof(DWORD)) + res = STG_E_READFAULT; + if (SUCCEEDED(res)) { StorageUtl_ReadDWord((BYTE *)&buffer, 0, nextBlockInChain); @@ -6386,7 +6407,7 @@ static ULONG SmallBlockChainStream_GetNextFreeBlock( /* * If we run out of space for the small block depot, enlarge it */ - if (SUCCEEDED(res)) + if (SUCCEEDED(res) && bytesRead == sizeof(DWORD)) { StorageUtl_ReadDWord((BYTE *)&buffer, 0, &nextBlockIndex); @@ -6482,12 +6503,21 @@ HRESULT SmallBlockChainStream_ReadAt( ULONG blockIndex; ULONG bytesReadFromBigBlockFile; BYTE* bufferWalker; + ULARGE_INTEGER stream_size; /* * This should never happen on a small block file. */ assert(offset.u.HighPart==0); + *bytesRead = 0; + + stream_size = SmallBlockChainStream_GetSize(This); + if (stream_size.QuadPart > offset.QuadPart) + size = min(stream_size.QuadPart - offset.QuadPart, size); + else + return S_OK; + /* * Find the first block in the stream that contains part of the buffer. */ @@ -6504,7 +6534,6 @@ HRESULT SmallBlockChainStream_ReadAt( /* * Start reading the buffer. */ - *bytesRead = 0; bufferWalker = buffer; while ( (size > 0) && (blockIndex != BLOCK_END_OF_CHAIN) ) @@ -6538,6 +6567,9 @@ HRESULT SmallBlockChainStream_ReadAt( if (FAILED(rc)) return rc; + if (!bytesReadFromBigBlockFile) + return STG_E_DOCFILECORRUPT; + /* * Step to the next big block. */ @@ -6551,7 +6583,7 @@ HRESULT SmallBlockChainStream_ReadAt( offsetInBlock = (offsetInBlock + bytesReadFromBigBlockFile) % This->parentStorage->smallBlockSize; } - return (size == 0) ? S_OK : STG_E_READFAULT; + return S_OK; } /****************************************************************************** diff --git a/dlls/ole32/tests/storage32.c b/dlls/ole32/tests/storage32.c index 8fa1afd..9bd8f33 100644 --- a/dlls/ole32/tests/storage32.c +++ b/dlls/ole32/tests/storage32.c @@ -315,6 +315,55 @@ static void test_storage_stream(void) r = IStream_Commit(stm, STGC_DEFAULT ); ok(r==S_OK, "failed to commit stream\n"); + /* Read past the end of the stream. */ + pos.QuadPart = 3; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 3, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 3, "read bytes past end of stream\n"); + pos.QuadPart = 10; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 10, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 0, "read bytes past end of stream\n"); + pos.QuadPart = 10000; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 10000, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 0, "read bytes past end of stream\n"); + + /* Convert to a big block stream, and read past the end. */ + p.QuadPart = 5000; + r = IStream_SetSize(stm,p); + ok(r==S_OK, "failed to set pos\n"); + pos.QuadPart = 4997; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 4997, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 3, "read bytes past end of stream\n"); + pos.QuadPart = 5001; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 5001, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 0, "read bytes past end of stream\n"); + pos.QuadPart = 10000; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 10000, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to read\n"); + ok(count == 0, "read bytes past end of stream\n"); + /* seek round a bit, reset the stream size */ pos.QuadPart = 0; r = IStream_Seek(stm, pos, 3, &p ); @@ -329,6 +378,16 @@ static void test_storage_stream(void) r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); ok(r==S_OK, "failed to seek stream\n"); ok(p.QuadPart == 10, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to set pos\n"); + ok(count == 0, "read bytes from empty stream\n"); + pos.QuadPart = 10000; + r = IStream_Seek(stm, pos, STREAM_SEEK_SET, &p ); + ok(r==S_OK, "failed to seek stream\n"); + ok(p.QuadPart == 10000, "at wrong place\n"); + r = IStream_Read(stm, buffer, sizeof buffer, &count ); + ok(r==S_OK, "failed to set pos\n"); + ok(count == 0, "read bytes from empty stream\n"); pos.QuadPart = 0; r = IStream_Seek(stm, pos, STREAM_SEEK_END, &p ); ok(r==S_OK, "failed to seek stream\n"); -- 1.6.3.3