I looked further into the problem I set out to fix, and realize that this patch is pretty terrible. The real bug is 22006 http://bugs.winehq.org/show_bug.cgi?id=22006 and affects the game Continuum http://appdb.winehq.org/appview.php?iVersionId=3703 basically, OpenProcess doesn't enfore ACLs
I am a user of the game, and the instructions that other users have provided involve a patch which allow the game to function. I was applying the patch from the git source, and wanted to try my hand at submitting a patch. The patch provided is described in the appdb page, and I thought I would improve it.
That patch (not the one I submitted) wouldn't leak any handles, since it returns null before attempting NtOpenProcess when (access & PROCESS_VM_WRITE). I'm not sure if that is proper behavior, but it still seems dangerous to me.
Long story short, please disregard this patch, and while I will continue my efforts, any suggestions on how to approach bug 22006 would be greatly appreciated.
Many thanks, Henry Blum
----- Original message -----
08.05.2010 9:10, Henry Blum wrote:
dlls/kernel32/process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 9a1f2f5..6dcad8f 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -2657,7 +2657,7 @@ HANDLE WINAPI OpenProcess( DWORD access, BOOL inherit, DWORD id ) if (GetVersion() & 0x80000000) access = PROCESS_ALL_ACCESS; status = NtOpenProcess(&handle, access, &attr, &cid); - if (status != STATUS_SUCCESS) + if (status != STATUS_SUCCESS || (access & PROCESS_VM_WRITE)) { SetLastError( RtlNtStatusToDosError(status) ); return NULL;
Wouldn't this pretend operation failed for every request with PROCESS_VM_WRITE, even if NtOpenProcess succeeded, and leak a handle? Which bug should it fix?