IFileOperation::CopyItems hookcode not work

c++ / delphi package - dll injection and api hooking
wali
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by wali »

Have you tested this on win7 or win10?
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
CopyItems.png (37.12 KiB) Viewed 16405 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
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by wali »

Ok, I will do a try.
wali
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by wali »

Does "latest 3.x" mean 3.1.13?
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by 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
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by madshi »

That seems kind of crazy. I don't even see why those 2 brackets would make any difference! :o Is it a compiler bug, maybe?
wali
Posts: 23
Joined: Tue Oct 11, 2016 7:05 am

Re: IFileOperation::CopyItems hookcode not work

Post by wali »

I guess it is not related to compiler.
It woks fine with 3.0
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: IFileOperation::CopyItems hookcode not work

Post by 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.
Post Reply