IFileOperation::CopyItems hookcode not work

[wali]

hi, @madshi
It's been a long time to wait for your response.
Our customer has also confronted this issue.
It would be extremely urgent solving it as quickly as your can.

Waiting for good news!

[madshi]

I'm really sorry for letting you wait so long. I'm working on it now and hope to have a solution in the next 1-3 days.

[madshi]

Ok, I've tested this on Windows 8.1 x64 now, and it seems to work just fine for me. I've slightly modified the test code, though. Here's what I've tested with:

Code: Select all

#include <windows.h>
#include <Shobjidl.h>
#include "madCHook_helper.h"

typedef HRESULT (WINAPI *PF_CoCreateInstance)(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID  *ppv);
PF_CoCreateInstance   Hooked_CoCreateInstance_Next = NULL;

HRESULT Hooked_COMCopyItems(IFileOperation* This, __RPC__in_opt IUnknown *punkItems, __RPC__in_opt IShellItem *psiDestinationFolder)
{
   OutputDebugStringW(L"testdll: Hooked_COMCopyItems hooked\n");
   return S_OK;
}

HRESULT WINAPI Hooked_CoCreateInstance(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID *ppv)
{
   HRESULT hr = Hooked_CoCreateInstance_Next(rclsid, pUnkOuter, dwClsContext, riid, ppv);
   if (SUCCEEDED(hr) &&
      (*ppv) != NULL &&
      ::IsEqualCLSID(rclsid, CLSID_FileOperation) &&
      ::IsEqualIID(riid, IID_IFileOperation))
   {       
      OutputDebugStringW(L"testdll: CoCreateInstance OK");
      IFileOperation*pObject = static_cast<IFileOperation*>(*ppv);
      LPVOID* pVTable = (*(LPVOID**)pObject);  // the v table of the object
      LPVOID pCopyItems = pVTable[17];
      LPVOID next_CopyItems = NULL;
      if(HookCode((LPVOID)pCopyItems,(PVOID)Hooked_COMCopyItems,(LPVOID*)&next_CopyItems) && next_CopyItems)
      {
         OutputDebugStringW(L"testdll: HookCode OK");
      }

   }
   return hr;
}

/***********************************************************************
// DllMain
//
// Entry point
***********************************************************************/
BOOL APIENTRY DllMain(
   HMODULE hModule,  //A handle to the DLL module. The value is the base address of the DLL.
   DWORD  ul_reason_for_call,
   LPVOID lpReserved
   )
{
   UNREFERENCED_PARAMETER(lpReserved);       
   if (DLL_PROCESS_ATTACH == ul_reason_for_call)
   {
      ::DisableThreadLibraryCalls(hModule);
      InitializeMadCHook();
      OutputDebugStringW(L"testdll loaded");
      if (HookAPI("ole32.dll", "CoCreateInstance", Hooked_CoCreateInstance, (PVOID*)&Hooked_CoCreateInstance_Next))
        OutputDebugStringW(L"testdll: HookAPI OK");
      else
        OutputDebugStringW(L"testdll: HookAPI failed");
      return TRUE;
   }
   else if (DLL_PROCESS_DETACH == ul_reason_for_call)
   {
      FinalizeMadCHook();
      OutputDebugStringW(L"testdll unloaded");
      return TRUE;
   }
   else
   {
      return TRUE;
   }

}/* DllMain */

And here's what I get when injecting the test.dll into the Explorer process and then copying a file on desktop by using Ctrl+C and Ctrl+V:

Code: Select all

[11152] testdll loaded
[11152] testdll: HookAPI OK
[11152] testdll: CoCreateInstance OK
[11152] testdll: HookCode OK
[11152] testdll: Hooked_COMCopyItems hooked

I've tested with the latest madCodeHook 3.x and 4.x builds, using madCHook64mt.lib:

http://madshi.net/madCollectionBeta.exe

[wali]

Have you tested this on win7 or win10?

[madshi]

No, I've tested only win8 x64. When I read it occurs on both win7 and win10 I thought it would also occur on win8. So you're saying it doesn't occur on win8? I'll recheck with win10 then.

[madshi]

Ok, I've tested on Windows 10 x64 now. And it works for me, too. Here's what I did:

1) I've created a simple injector exe, with hard coded Explorer.exe ProcessID, to make things simple:

Code: Select all

program InjectExplorer;

uses Windows, madCodeHook;

var ph : THandle;
begin
  EnableAllPrivileges;
  ph := OpenProcess(PROCESS_ALL_ACCESS, false, 3404);
  if ph <> 0 then
    if MessageBox(0, 'inject?', 'question', MB_YESNO) = IDYES then
      InjectLibrary('Test.dll', ph)
    else
      UninjectLibrary('Test.dll', ph);
end.

2) I've compiled your test project as 64bit Release build, using both the latest 3.x and 4.x madCodeHook builds. It works with both for me.

3) I've copied the "InjectExplorer.exe" and "Test.dll" to the desktop of the Windows 10 x64 VM.

4) I've executed InjectExplorer.exe, injected the dll, then copied a file on the desktop. See here:

CopyItems.png (37.21 KiB) Viewed 2079 times

I'm not sure if I changed anything important in the source code or project settings, so I'm attaching my test project folder here. You can try the "x64\Release\Test.dll" file to see if it works for you.

[madshi]

P.S: The forum doesn't seem to like attachments with DLL files in it, so I've uploaded it to my server here:

http://madshi.net/CopyFileTest.zip

[wali]

Ok, I will do a try.

[wali]

Does "latest 3.x" mean 3.1.13?

[madshi]

I've used my work-in-progress sources, but I don't think there have been any meaningful changes compared to 3.1.13.

[wali]

Code: Select all

#include <windows.h>
#include <Shobjidl.h>
#include "madCHook_helper.h"


typedef HRESULT (WINAPI *PF_CoCreateInstance)(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID  *ppv);
PF_CoCreateInstance   Hooked_CoCreateInstance_Next = NULL;

HRESULT Hooked_COMRenameItem(IFileOperation * This, IShellItem *psiDestinationFolder, LPCWSTR pszNewName, IFileOperationProgressSink *pfopsItem)
{
   OutputDebugStringW(L"testdll: Hooked_COMRenameItem hooked\n");
   return S_OK;
}

HRESULT Hooked_COMCopyItems(IFileOperation* This, __RPC__in_opt IUnknown *punkItems, __RPC__in_opt IShellItem *psiDestinationFolder)
{
   OutputDebugStringW(L"testdll: Hooked_COMCopyItems hooked\n");
   return S_OK;
}

HRESULT WINAPI Hooked_CoCreateInstance(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID *ppv)
{
   HRESULT hr = Hooked_CoCreateInstance_Next(rclsid, pUnkOuter, dwClsContext, riid, ppv);
   if (SUCCEEDED(hr) &&
      (*ppv) != NULL &&
      ::IsEqualCLSID(rclsid, CLSID_FileOperation) &&
      ::IsEqualIID(riid, IID_IFileOperation))
   {       
      OutputDebugStringW(L"testdll: Hooked_CoCreateInstance IFileOperation hooked\n");

      IFileOperation*pObject = static_cast<IFileOperation*>(*ppv);
      LPVOID* pVTable = (*(LPVOID**)pObject);  // the v table of the object

      [color=#BF4000]{   
         LPVOID pRenameItem = pVTable[12];

         LPVOID next_RenameItem = NULL;
         if(HookCode((LPVOID)pRenameItem,(PVOID)Hooked_COMRenameItem,(LPVOID*)&next_RenameItem) && next_RenameItem)
         {
            OutputDebugStringW(L"testdll: Hook COMRenameItem OK");
         }
      }

      {
         LPVOID pCopyItems = pVTable[17];

         LPVOID next_CopyItems = NULL;
         if(HookCode((LPVOID)pCopyItems,(PVOID)Hooked_COMCopyItems,(LPVOID*)&next_CopyItems) && next_CopyItems)
         {
            OutputDebugStringW(L"testdll: Hook COMCopyItems OK");
         }
      }[/color]
   }
   return hr;
}




Hi madshi
We have reproduced the bug. Please check the code above.
Here are some differences:
1.use 'hookcode' twice
2.add two brackets
result:
'Hooked_COMRenameItem' is called while 'Hooked_COMCopyItems' not

[wali]

see the whole code of dllmain.cpp below.
Please take attention to the comments in source file, and compare with two conditions (with/without brackets).

Code: Select all

#include <windows.h>
#include <Shobjidl.h>
#include "madCHook_helper.h"


typedef HRESULT (WINAPI *PF_CoCreateInstance)(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID  *ppv);
PF_CoCreateInstance   Hooked_CoCreateInstance_Next = NULL;

HRESULT Hooked_COMRenameItem(IFileOperation * This, IShellItem *psiDestinationFolder, LPCWSTR pszNewName, IFileOperationProgressSink *pfopsItem)
{
   OutputDebugStringW(L"testdll: Hooked_COMRenameItem hooked\n");
   return S_OK;
}

HRESULT Hooked_COMCopyItems(IFileOperation* This, __RPC__in_opt IUnknown *punkItems, __RPC__in_opt IShellItem *psiDestinationFolder)
{
   OutputDebugStringW(L"testdll: Hooked_COMCopyItems hooked\n");
   return S_OK;
}

HRESULT WINAPI Hooked_CoCreateInstance(REFCLSID rclsid, LPUNKNOWN pUnkOuter, DWORD dwClsContext, REFIID riid, LPVOID *ppv)
{
   HRESULT hr = Hooked_CoCreateInstance_Next(rclsid, pUnkOuter, dwClsContext, riid, ppv);
   if (SUCCEEDED(hr) &&
      (*ppv) != NULL &&
      ::IsEqualCLSID(rclsid, CLSID_FileOperation) &&
      ::IsEqualIID(riid, IID_IFileOperation))
   {       
      OutputDebugStringW(L"testdll: Hooked_CoCreateInstance IFileOperation hooked\n");

      IFileOperation*pObject = static_cast<IFileOperation*>(*ppv);
      LPVOID* pVTable = (*(LPVOID**)pObject);  // the v table of the object

      {   
         LPVOID pRenameItem = pVTable[12];

         LPVOID next_RenameItem = NULL;
         if(HookCode((LPVOID)pRenameItem,(PVOID)Hooked_COMRenameItem,(LPVOID*)&next_RenameItem) && next_RenameItem)
         {
            OutputDebugStringW(L"testdll: Hook COMRenameItem OK");
         }
      }

       
        //attention:
        //if this bracket is removed, two functions are called
        //while if not, function 'Hooked_COMCopyItems' is not called
      {
         LPVOID pCopyItems = pVTable[17];

         LPVOID next_CopyItems = NULL;
         if(HookCode((LPVOID)pCopyItems,(PVOID)Hooked_COMCopyItems,(LPVOID*)&next_CopyItems) && next_CopyItems)
         {
            OutputDebugStringW(L"testdll: Hook COMCopyItems OK");
         }
      }
   }
   return hr;
}

/***********************************************************************
// DllMain
//
// Entry point
***********************************************************************/
BOOL APIENTRY DllMain(
   HMODULE hModule,  //A handle to the DLL module. The value is the base address of the DLL.
   DWORD  ul_reason_for_call,
   LPVOID lpReserved
   )
{
   UNREFERENCED_PARAMETER(lpReserved);       
   if (DLL_PROCESS_ATTACH == ul_reason_for_call)
   {
      OutputDebugStringW(L"testdll loaded");

      ::DisableThreadLibraryCalls(hModule);
      InitializeMadCHook();

      if (HookAPI("ole32", "CoCreateInstance", Hooked_CoCreateInstance, (PVOID*)&Hooked_CoCreateInstance_Next))
      {
         OutputDebugStringW(L"testdll: Hook CoCreateInstance OK");
      }
      else
      {
         OutputDebugStringW(L"testdll: Hook CoCreateInstance failed");
      }

      return TRUE;
   }
   else if (DLL_PROCESS_DETACH == ul_reason_for_call)
   {
      FinalizeMadCHook();

      OutputDebugStringW(L"testdll unloaded");
      return TRUE;
   }
   else
   {
      return TRUE;
   }

}/* DllMain */



[madshi]

That seems kind of crazy. I don't even see why those 2 brackets would make any difference! Is it a compiler bug, maybe?

[wali]

I guess it is not related to compiler.
It woks fine with 3.0

[madshi]

I haven't noticed this before, not sure why, but there's a serious error in your source code:

madCodeHook requires your "next" function variable to be a global variable, for multiple reasons. Recently several madCodeHook users started using local "next" variables. I'm not sure why, it's as very new trend. Haven't seen this in the 10 years before, but in the last couple of months, I've seen it at least 3 times now.

The problem occurs because the compiler reuses the same address on the stack for both the local "next_RenameItem" and "next_CopyItems" variables, when using those brackets. Without the brackets, the compiler uses different stack addresses for the 2 local variables. Now older madCodeHook builds might not have cared, they simply installed the API hook multiple times, with the same "next" variable. However, the latest madCodeHook version is careful not to install multiple hooks with the same "next" variable address. This is why the 2nd hook doesn't work. madCodeHook detects that the same "next" variable is already in use.

The fix is very simple: Make those "next" declarations global. That way each "next" variable gets a different address, and everything should work just fine.

If you look through my demos, "next" declarations are ALWAYS global. They have to be, otherwise "next" points to the stack, which can introduce all sorts of trouble.

BTW, Hooked_CoCreateInstance might be called multiple times, and you shouldn't really try to install the same API hook multiple times with the same "next" variable. So in order to make the code cleaner I'd strongly suggest that you only call HookCode() if the global "next" variable is NULL.