Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64dbg 'blocks' file rename and write operations on the debuggee #2504

Closed
cw2k opened this issue Nov 3, 2020 · 9 comments · Fixed by #3234
Closed

x64dbg 'blocks' file rename and write operations on the debuggee #2504

cw2k opened this issue Nov 3, 2020 · 9 comments · Fixed by #3234
Labels
bug The issue describes a bug. It does not mean the bug has been reproduced by a developer.

Comments

@cw2k
Copy link
Contributor

cw2k commented Nov 3, 2020

What is the matter?

x64dbg should always open file like this:
GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE

Why FILE_SHARE_WRITE and FILE_SHARE_DELETE when I just need READ access?

Even if you wanna to open the file for read-only access you should allow others also to write the file and to do so you have to specify for the sharing flags FILE_SHARE_WRITE. For renaming FILE_SHARE_DELETE is required.
So in the context of CreateFile() this means:

dwDesiredAccess=GENERIC_READ
dwShareMode=FILE_SHARE_READ | **FILE_SHARE_WRITE  | FILE_SHARE_DELETE**

Why that is important? Well let's do some example.
0. Just check if you can rename calc.exe to mycalc.exe. (Rename it back to calc.exe)

  1. start calc.exe. Keep it open and try rename it to mycalc.exe. That should work
  2. Attach x64dbg to calc.exe try renaming it now. You get an error and can't do it.
  3. Detach x64dbg from calc.exe try renaming it now. Now it works again.

Example

The nice thing about FILE_SHARE_WRITE | FILE_SHARE_DELETE is that the patcher will be able to save changes to the current running debuggee.

So for this change the 'Backup' algo of the patcher like this.

  1. RENAME Target.exe -> Target.exe.bak
  2. Copy Target.exe.bak -> Target.exe.
    Now you have Target.exe and Target.exe.bak as intented.
    The difference is now that you will be able to open Target.exe for write access.
    ( Well write access for Target.exe.bak will fail however that is not we needed)

Ideas for a SelfcheckCheater plugin

Btw the rename trick can also be used to cheat file/filename based self checks.
Let's there be Target.exe and the unmodifed copy Target.org
When debugger stops at first system breakpoint it'll just swap Target.exe Target.org. When it quits I'll swap it back.

Or to be more resilienten and safe change that algo like this:
Before load check if there is a Target.swap. If that is the case debugger somehow crash or was unable to swap back the files.
So run 'onQuit' handler now that does a proper swapback.
When loaded the actual Target.exe gets renamed to Target.swap.
On quit that Target.swap gets Target.exe and the Target.exe is named to Target.org

So maybe it's time for a SelfcheckCheater plugin. Would it possible to do so with current plugin API?

@mrexodia
Copy link
Member

mrexodia commented Nov 6, 2020

Yeah this is more or less a known way to detect x64dbg (x64dbg/TitanEngine#5). But this is technically an issue in TitanEngine. To fix this all instances that use the existing file mapping need to be changed to directly map the file and cache all the relevant details, because if you allow a file to write to itself it is no longer 'trusted' input (eg the file could modify its own header on disk while running to prevent patching).

For a plugin, just use minhook to hook CreateFileW and add the sharing attributes you want 🙂

@cw2k
Copy link
Contributor Author

cw2k commented Nov 8, 2020

Hmm hooking CreateFileW to add the sharing attributes I want. Well that will work.
However for closed source plugins I will try to do some manual binary patching at first to accomplish the same more direct and easy.

because if you allow a file to write to itself it is no longer 'trusted' input

Maybe in some rare cases that will be needed. So implement that as an advanced option 'Deny shared access to opened file.'that can be enabled if someone really need such a 'trusted' input. However in first place (default setting) it should allow SHARED_READ, SHARED_WRITE and SHARED_DELETE.

I patched x64dbg\x64\TitanEngine.dll
45 8D ?? 03 ?? ?? ?? ?? 80 -> 45 8D ?? 07 ?? ?? ?? ?? 80
at 2 locations. Now it works. However would be nice to also have that in the code base so I don't need that 'binary fix' anymore.

@mrexodia
Copy link
Member

mrexodia commented Nov 8, 2020

Maybe in some rare cases that will be needed.

In some rare cases it's needed to share the file, safety is far more important than allowing shared writes. I think a CreateFileW hook will work fine if you really want to allow sharing, at least until I get the chance to completely refactor the module system to not keep a handle open to the file at all.

@cw2k
Copy link
Contributor Author

cw2k commented Nov 9, 2020

I see. Would you agree to with some option to toggle shared writes on/off?

@mrexodia
Copy link
Member

mrexodia commented Nov 9, 2020 via email

@cw2k
Copy link
Contributor Author

cw2k commented Nov 9, 2020

Okay so my rough plan for now will be to write a x64dbg-plugin that will use minhook to hook CreateFileW to hack in shared writes. :^)

@mrexodia
Copy link
Member

mrexodia commented Nov 9, 2020

If you're interested here is my template for minhook to make hooking things trivially easy:

#include "ntdll/ntdll.h"
#include <Windows.h>
#include "debug.h"
#include "MinHook/MinHook.h"
#include "vid.h"

extern "C" __declspec(dllexport) void surprise() { }

struct Hook
{
	const wchar_t* pszModule;
	const char* pszProcName;
	PVOID pDetour;
	PVOID* ppOriginal;
};

// Can's magic macro for passing macrofn(ESCAPE(T<int, int>))
#define ESCAPE(...) __VA_ARGS__

#pragma section(".hooks$1",long,read)
#pragma section(".hooks$2",long,read)
#pragma section(".hooks$3",long,read)
#pragma comment(linker, "/merge:hooks=.rdata")

// Can's magic2
__declspec(allocate(".hooks$1")) const Hook hooks_begin;
__declspec(allocate(".hooks$3")) const Hook hooks_end;

// You likely forgot about WINAPI
// error C2373: 'hook_Function': redefinition; different type modifiers
#define HOOK(Dll, ReturnType, Function) \
	static decltype(&Function) original_ ## Function; \
	static decltype(Function) hook_ ## Function; \
	__declspec(allocate(".hooks$2")) const Hook dupa ## Function = { L ### Function, #Function, hook_ ## Function, (LPVOID*)&original_ ## Function }; \
	static ReturnType hook_ ## Function

HOOK(vid.dll, BOOL WINAPI, VidRegisterCpuidHandler)(
	__in PT_HANDLE Partition,
	__in VID_PROCESSOR_INDEX ProcessorIndex,
	__in VID_QUEUE_HANDLE MessageQueue,
	__in UINT32 CpuidFunction,
	__in PVOID UserContext,
	__out HANDLER_REF* HandlerRef
)
{
	dlog();
	return original_VidRegisterCpuidHandler(Partition, ProcessorIndex, MessageQueue, CpuidFunction, UserContext, HandlerRef);
}

#include <iterator>

BOOL WINAPI DllMain(
	_In_ HINSTANCE hinstDLL,
	_In_ DWORD     fdwReason,
	_In_ LPVOID    lpvReserved
	)
{
	dlog();
	if (fdwReason == DLL_PROCESS_ATTACH)
	{
		if (MH_Initialize() != MH_OK)
		{
			dlogp("MH_Initialize failed");
			return FALSE;
		}
		for (auto hook = std::next(&hooks_begin); hook != &hooks_end; ++hook)
		{
			if (!MH_CreateHookApi(hook->pszModule, hook->pszProcName, hook->pDetour, hook->ppOriginal))
			{
				dlogp("MH_CreateHook(%s) failed", hook->pszProcName);
				return FALSE;
			}
		}
		if (MH_EnableHook(MH_ALL_HOOKS) != MH_OK)
		{
			dlogp("MH_EnableHook failed");
			return FALSE;
		}
	}
	return TRUE;
}

@cw2k
Copy link
Contributor Author

cw2k commented Nov 10, 2020

Wow nice.
What a nice 'surprise'.
Nice code.

Hmm I just out of curiosity I wonders that surprise() dllexport was for?

@mrexodia
Copy link
Member

mrexodia commented Nov 10, 2020 via email

@mrexodia mrexodia added the bug The issue describes a bug. It does not mean the bug has been reproduced by a developer. label Jan 5, 2022
mrexodia added a commit that referenced this issue Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue describes a bug. It does not mean the bug has been reproduced by a developer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants