From 9d15f0874bfd1fbf00ba9d8784315c06efea4579 Mon Sep 17 00:00:00 2001 From: Vincent Povirk Date: Thu, 2 Jun 2011 14:34:53 -0500 Subject: [PATCH] ole32: Avoid opening source storage in IStorage::CopyTo. --- dlls/ole32/storage32.c | 279 +++++++++++++++++++++++------------------ dlls/ole32/tests/storage32.c | 2 +- 2 files changed, 157 insertions(+), 124 deletions(-) diff --git a/dlls/ole32/storage32.c b/dlls/ole32/storage32.c index 44adaed..e983bf3 100644 --- a/dlls/ole32/storage32.c +++ b/dlls/ole32/storage32.c @@ -87,6 +87,9 @@ struct StorageInternalImpl }; typedef struct StorageInternalImpl StorageInternalImpl; +static const IStorageVtbl TransactedSnapshotImpl_Vtbl; +static const IStorageVtbl Storage32InternalImpl_Vtbl; + /* Method definitions for the Storage32InternalImpl class. */ static StorageInternalImpl* StorageInternalImpl_Construct(StorageBaseImpl* parentStorage, DWORD openFlags, DirRef storageDirEntry); @@ -1600,163 +1603,88 @@ static HRESULT findTreeParent(StorageBaseImpl *storage, DirRef storageEntry, } -/************************************************************************* - * CopyTo (IStorage) - */ -static HRESULT WINAPI StorageBaseImpl_CopyTo( - IStorage* iface, - DWORD ciidExclude, /* [in] */ - const IID* rgiidExclude, /* [size_is][unique][in] */ - SNB snbExclude, /* [unique][in] */ - IStorage* pstgDest) /* [unique][in] */ -{ - StorageBaseImpl* const This=(StorageBaseImpl*)iface; +static HRESULT StorageBaseImpl_CopyStorageEntryTo(StorageBaseImpl *This, + DirRef srcEntry, BOOL skip_storage, BOOL skip_stream, + SNB snbExclude, IStorage *pstgDest); - IEnumSTATSTG *elements = 0; - STATSTG curElement, strStat; - HRESULT hr; - IStorage *pstgTmp, *pstgChild; - IStream *pstrTmp, *pstrChild; - DirRef srcEntryRef; - DirEntry srcEntry; - BOOL skip = FALSE, skip_storage = FALSE, skip_stream = FALSE; - int i; - - TRACE("(%p, %d, %p, %p, %p)\n", - iface, ciidExclude, rgiidExclude, - snbExclude, pstgDest); +static HRESULT StorageBaseImpl_CopyChildEntryTo(StorageBaseImpl *This, + DirRef srcEntry, BOOL skip_storage, BOOL skip_stream, + SNB snbExclude, IStorage *pstgDest) +{ + DirEntry data; + HRESULT hr; + BOOL skip = FALSE; + IStorage *pstgTmp; + IStream *pstrChild, *pstrTmp; + STATSTG strStat; - if ( pstgDest == 0 ) - return STG_E_INVALIDPOINTER; + if (srcEntry == DIRENTRY_NULL) + return S_OK; - /* - * Enumerate the elements - */ - hr = IStorage_EnumElements( iface, 0, 0, 0, &elements ); + hr = StorageBaseImpl_ReadDirEntry( This, srcEntry, &data ); - if ( hr != S_OK ) + if (FAILED(hr)) return hr; - /* - * set the class ID - */ - IStorage_Stat( iface, &curElement, STATFLAG_NONAME); - IStorage_SetClass( pstgDest, &curElement.clsid ); - - for(i = 0; i < ciidExclude; ++i) + if ( snbExclude ) { - if(IsEqualGUID(&IID_IStorage, &rgiidExclude[i])) - skip_storage = TRUE; - else if(IsEqualGUID(&IID_IStream, &rgiidExclude[i])) - skip_stream = TRUE; - else - WARN("Unknown excluded GUID: %s\n", debugstr_guid(&rgiidExclude[i])); - } - - do - { - /* - * Obtain the next element - */ - hr = IEnumSTATSTG_Next( elements, 1, &curElement, NULL ); + WCHAR **snb = snbExclude; - if ( hr == S_FALSE ) + while ( *snb != NULL && !skip ) { - hr = S_OK; /* done, every element has been copied */ - break; + if ( lstrcmpW(data.name, *snb) == 0 ) + skip = TRUE; + ++snb; } + } - if ( snbExclude ) - { - WCHAR **snb = snbExclude; - skip = FALSE; - while ( *snb != NULL && !skip ) - { - if ( lstrcmpW(curElement.pwcsName, *snb) == 0 ) - skip = TRUE; - ++snb; - } - } - - if ( skip ) - goto cleanup; - - if (curElement.type == STGTY_STORAGE) + if (!skip) + { + if (data.stgType == STGTY_STORAGE && !skip_storage) { - if(skip_storage) - goto cleanup; - - /* - * open child source storage - */ - hr = IStorage_OpenStorage( iface, curElement.pwcsName, NULL, - STGM_READ|STGM_SHARE_EXCLUSIVE, - NULL, 0, &pstgChild ); - - if (hr != S_OK) - goto cleanup; - /* * create a new storage in destination storage */ - hr = IStorage_CreateStorage( pstgDest, curElement.pwcsName, + hr = IStorage_CreateStorage( pstgDest, data.name, STGM_FAILIFTHERE|STGM_WRITE|STGM_SHARE_EXCLUSIVE, - 0, 0, + 0, 0, &pstgTmp ); + /* * if it already exist, don't create a new one use this one */ if (hr == STG_E_FILEALREADYEXISTS) { - hr = IStorage_OpenStorage( pstgDest, curElement.pwcsName, NULL, + hr = IStorage_OpenStorage( pstgDest, data.name, NULL, STGM_WRITE|STGM_SHARE_EXCLUSIVE, NULL, 0, &pstgTmp ); } - if (hr == S_OK) + if (SUCCEEDED(hr)) { - /* - * do the copy recursively - */ - hr = IStorage_CopyTo( pstgChild, ciidExclude, rgiidExclude, - NULL, pstgTmp ); + hr = StorageBaseImpl_CopyStorageEntryTo( This, srcEntry, skip_storage, + skip_stream, NULL, pstgTmp ); - IStorage_Release( pstgTmp ); + IStorage_Release(pstgTmp); } - - IStorage_Release( pstgChild ); } - else if (curElement.type == STGTY_STREAM) + else if (data.stgType == STGTY_STREAM && !skip_stream) { - if(skip_stream) - goto cleanup; - /* * create a new stream in destination storage. If the stream already * exist, it will be deleted and a new one will be created. */ - hr = IStorage_CreateStream( pstgDest, curElement.pwcsName, + hr = IStorage_CreateStream( pstgDest, data.name, STGM_CREATE|STGM_WRITE|STGM_SHARE_EXCLUSIVE, 0, 0, &pstrTmp ); - if (hr != S_OK) - goto cleanup; - /* * open child stream storage. This operation must succeed even if the * stream is already open, so we use internal functions to do it. */ - srcEntryRef = findElement( This, This->storageDirEntry, curElement.pwcsName, - &srcEntry); - if (!srcEntryRef) - { - ERR("source stream not found\n"); - hr = STG_E_DOCFILECORRUPT; - } - if (hr == S_OK) { - pstrChild = (IStream*)StgStreamImpl_Construct(This, STGM_READ|STGM_SHARE_EXCLUSIVE, srcEntryRef); + pstrChild = (IStream*)StgStreamImpl_Construct(This, STGM_READ|STGM_SHARE_EXCLUSIVE, srcEntry); if (pstrChild) IStream_AddRef(pstrChild); else @@ -1786,21 +1714,126 @@ static HRESULT WINAPI StorageBaseImpl_CopyTo( IStream_Release( pstrTmp ); } + } + + /* copy siblings */ + if (SUCCEEDED(hr)) + hr = StorageBaseImpl_CopyChildEntryTo( This, data.leftChild, skip_storage, + skip_stream, snbExclude, pstgDest ); + + if (SUCCEEDED(hr)) + hr = StorageBaseImpl_CopyChildEntryTo( This, data.rightChild, skip_storage, + skip_stream, snbExclude, pstgDest ); + + return hr; +} + +static HRESULT StorageBaseImpl_CopyStorageEntryTo(StorageBaseImpl *This, + DirRef srcEntry, BOOL skip_storage, BOOL skip_stream, + SNB snbExclude, IStorage *pstgDest) +{ + DirEntry data; + HRESULT hr; + + hr = StorageBaseImpl_ReadDirEntry( This, srcEntry, &data ); + + if (SUCCEEDED(hr)) + hr = IStorage_SetClass( pstgDest, &data.clsid ); + + if (SUCCEEDED(hr)) + hr = StorageBaseImpl_CopyChildEntryTo( This, data.dirRootEntry, skip_storage, + skip_stream, snbExclude, pstgDest ); + + return hr; +} + +/************************************************************************* + * CopyTo (IStorage) + */ +static HRESULT WINAPI StorageBaseImpl_CopyTo( + IStorage* iface, + DWORD ciidExclude, /* [in] */ + const IID* rgiidExclude, /* [size_is][unique][in] */ + SNB snbExclude, /* [unique][in] */ + IStorage* pstgDest) /* [unique][in] */ +{ + StorageBaseImpl* const This=(StorageBaseImpl*)iface; + + BOOL skip_storage = FALSE, skip_stream = FALSE; + int i; + + TRACE("(%p, %d, %p, %p, %p)\n", + iface, ciidExclude, rgiidExclude, + snbExclude, pstgDest); + + if ( pstgDest == 0 ) + return STG_E_INVALIDPOINTER; + + for(i = 0; i < ciidExclude; ++i) + { + if(IsEqualGUID(&IID_IStorage, &rgiidExclude[i])) + skip_storage = TRUE; + else if(IsEqualGUID(&IID_IStream, &rgiidExclude[i])) + skip_stream = TRUE; else - { - WARN("unknown element type: %d\n", curElement.type); + WARN("Unknown excluded GUID: %s\n", debugstr_guid(&rgiidExclude[i])); + } + + if (!skip_storage) + { + /* Give up early if it looks like this would be infinitely recursive. + * Oddly enough, this includes some cases that aren't really recursive, like + * copying to a transacted child. */ + IStorage *pstgDestAncestor = pstgDest; + IStorage *pstgDestAncestorChild = NULL; + + /* Go up the chain from the destination until we find the source storage. */ + while (pstgDestAncestor != iface) { + pstgDestAncestorChild = pstgDest; + + if (pstgDestAncestor->lpVtbl == &TransactedSnapshotImpl_Vtbl) + { + TransactedSnapshotImpl *impl = (TransactedSnapshotImpl*) pstgDestAncestor; + + pstgDestAncestor = (IStorage*)impl->transactedParent; + } + else if (pstgDestAncestor->lpVtbl == &Storage32InternalImpl_Vtbl) + { + StorageInternalImpl *impl = (StorageInternalImpl*) pstgDestAncestor; + + pstgDestAncestor = (IStorage*)impl->parentStorage; + } + else + break; } -cleanup: - CoTaskMemFree(curElement.pwcsName); - } while (hr == S_OK); + if (pstgDestAncestor == iface) + { + BOOL fail = TRUE; - /* - * Clean-up - */ - IEnumSTATSTG_Release(elements); + if (pstgDestAncestorChild && snbExclude) + { + StorageBaseImpl *ancestorChildBase = (StorageBaseImpl*)pstgDestAncestorChild; + DirEntry data; + WCHAR **snb = snbExclude; - return hr; + StorageBaseImpl_ReadDirEntry(ancestorChildBase, ancestorChildBase->storageDirEntry, &data); + + while ( *snb != NULL && fail ) + { + if ( lstrcmpW(data.name, *snb) == 0 ) + fail = FALSE; + ++snb; + } + } + + if (fail) + return STG_E_ACCESSDENIED; + } + } + + return StorageBaseImpl_CopyStorageEntryTo( This, This->storageDirEntry, + skip_storage, skip_stream, snbExclude, pstgDest ); } /************************************************************************* diff --git a/dlls/ole32/tests/storage32.c b/dlls/ole32/tests/storage32.c index 83d5d2f..69103b5 100644 --- a/dlls/ole32/tests/storage32.c +++ b/dlls/ole32/tests/storage32.c @@ -2881,7 +2881,7 @@ static void test_copyto_locking(void) /* Try to copy the storage while the substorage is open */ r = IStorage_CopyTo(stg2, 0, NULL, NULL, stg3); - todo_wine ok(r==S_OK, "IStorage->CopyTo failed, hr=%08x\n", r); + ok(r==S_OK, "IStorage->CopyTo failed, hr=%08x\n", r); IStorage_Release(stg4); IStorage_Release(stg3); -- 1.7.1