Feature: Add DetourFindRemotePayload and improve other payload-related methods

[sylveon]

Other improvements are:

• Makes the pcbData parameter in DetourFindPayload and DetourFindPayloadEx optional, so that if an application only needs to search for the presence of a payload, they can ignore the size by passing nullptr.
• Makes the pvData parameter in DetourCopyPayloadToProcess const, so that a pointer to a const C++ object can be passed instead of the object needing to be const_casted or being non-const.
• Adds DetourCopyPayloadToProcessEx, which has the same interface than DetourCopyPayloadToProcess, but it returns the address of the payload in the remote module, if the program later wants to write to it.

Fixes #79

From basic testing, it seems to work for me. I can apply corrections and add a sample if required (although finding a trivial use case for those APIs would be required)

[sylveon]

@dtarditi could this and other PRs/issues be addressed? I'd like at some point to start using this in production (it's been working flawlessly in development)

[dtarditi]

@sylveon, what is the motivation for wanting to add DetourFindRemotePayload? The other changes look straightforward. It isn't clear to me how this new function would help with the core goal of detours, which is allow ppeople to instrument and monitor and API calls.

[sylveon]

My use-case is described in #79.

Basically, I use the payload as a flag to check if the DLL is actually loaded in the process I want to detour API calls (much like the current CreateProcessWithDll code does) and I may use it in the future to transmit information to the DLL when it is injected (like a shared handle or a pointer to a memory map).

However, the way my app works, the user can close and reopen it at will, so the DLL and detour may be removed (as in, completely unloaded from the target process's address space) and added multiple times within the lifetime of the target process.

Without this API, I cannot know if a payload already exists in the target process, so I always have to create a new payload, making its potential future purpose of transmitting information useless since there is not guarantee about which payload DetourFindPayload will give you if there are multiple payloads with the same GUID as well as new instances not being able to write to it, and it also leaks memory each time my app is closed and relaunched due to Detours not providing a way to deallocate the payload from the target process.

With this API, I can instead just find an existing payload (or create a new payload if there is none yet), not having to allocate a new one each time, and then edit the payload's content using WriteProcessMemory calls.

This function can very well be written standalone, but since it taps into implementation details of Detours, which might change in the future, I think that having it as part of the core codebase would be better.

[sylveon]

Note that my commits became unverified due to me changing my GPG key, you can ignore that.

[dtarditi]

@sylveon, this use case makes sense. Could you add a test case for the new API function?

[sylveon]

@dtarditi I fixed a bug related to memory allocation (used delete instead of delete[], fixed that issue by avoiding heap allocations entirely), and added a sample. The sample is not representative of my real world use, but it should be good enough to test and demonstrate the API. What it does is:

• creates a child process
• creates a payload in that child process containing a handle of the parent process
• parent process adds a payload to itself containing zeroes using DetourCopyPayloadToProcessEx and remembers the memory address of the payload data
• child process grabs the handle to the parent process using DetourFindPayloadEx
• child process finds the payload in the parent process using DetourFindRemotePayload and fills it with random data using WriteProcessMemory
• child process exits using the same random data as exit code
• parent process makes sure the payload data matches the child process exit code, and returns with 1 if it doesn't

[bgianfo]

It you don't mind, it would be nice if you could include the markdown for the WIKI documentation pages for these new API's in this PR (just in the PR description, or as comments) we can just paste them in once we merge.

[sylveon]

https://gist.github.com/sylveon/0a38efaa506ccebf5c91c7e4e32deaf5 I've uploaded my wiki updates here

[bgianfo]

@dtarditi, do you think this needs more work? Or is it ready to merge?

[dtarditi]

Looks good to me.

[bgianfo]

@sylveon I would like to include this in the next release can you:

• Fix up the merge conflicts
• Add some basic unit test coverage of this in the new unit tests.

[sylveon]

Yeah I was actually planning to take a look at the conflicts tonight. I'll see for the unit tests as well.

[sylveon]

I resolved conflicts and added unit test @bgianfo
While I was at it I noticed quite a few random tabs where present in source code, so I find & replace'd them, which will help in preparation to #181

[bgianfo]

Sorry can you submit the tab commit as a separate PR? Since the repository is configured to only allow squash commits, that's going to make this PR's commit diff pretty noisy and messy. ☹

[sylveon]

Reverted that commit for now

[sylveon]

There seems to be some contamination between PR builds, the builds are complaining about tests for DetourFindPayloadEx already existing in test_modules_api.cpp at line 623, but the file isn't even that long in my branch.

[sylveon]

Looks like I've got conflicts to resolve anyways - will do tonight

[bgianfo]

Thanks again for the contribution @sylveon!

[bgianfo]

I merged all of the doc changes that you provided in your gist as well.
See: https://github.com/microsoft/Detours/wiki/_compare/6c0fef6d375e2eebf558be1f6c3c8a817a4121a4...da14a2afdadfa3c7a4018b7e44485fb2825ce00c