[PATCH v5 0/4] MR6767: WriteProcessMemory() without PROCESS_QUERY_INFORMATION access right.

[eric pouech (＠epo)]

This is an alternate solution to MR 6761 [1]
(actually is based on comment in that MR).

WriteProcessMemory() should succeed when process handle doesn't have
PROCESS_QUERY_INFORMATION access right set (test attached).
Plus some additional cleanup in test for ReadProcessMemory().

Feedback from Proton says that this patch solves the reported game issue.

[1] https://gitlab.winehq.org/wine/wine/-/merge_requests/6761

--
  v5: server: Amend process rights mapping.
      advapi32: Test some other cases of process access rights mapping.
      advapi32/tests: Fix typo in manifest constant.
      kernel32/tests: Don't hardcode page size in buffer size.

https://gitlab.winehq.org/wine/wine/-/merge_requests/6767

[PATCH v5 1/4] kernel32/tests: Don't hardcode page size in buffer size.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 dlls/kernel32/tests/virtual.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c
index 12841133c05..cfbfcac0950 100644
--- a/dlls/kernel32/tests/virtual.c
+++ b/dlls/kernel32/tests/virtual.c
@@ -4368,13 +4368,15 @@ static void test_PrefetchVirtualMemory(void)

static void test_ReadProcessMemory(void)
 {
-    BYTE buf[0x2000];
+    BYTE *buf;
     DWORD old_prot;
     SIZE_T copied;
     HANDLE hproc;
     void *ptr;
     BOOL ret;

+    buf = malloc(2 * si.dwPageSize);
+    ok(buf != NULL, "OOM\n");
     ret = DuplicateHandle(GetCurrentProcess(), GetCurrentProcess(), GetCurrentProcess(),
             &hproc, 0, FALSE, DUPLICATE_SAME_ACCESS);
     ok(ret, "DuplicateHandle failed %lu\n", GetLastError());
@@ -4403,6 +4405,7 @@ static void test_ReadProcessMemory(void)
     ret = VirtualFree(ptr, 0, MEM_RELEASE);
     ok(ret, "VirtualFree failed %lu\n", GetLastError());
     CloseHandle(hproc);
+    free(buf);
 }

START_TEST(virtual)

[PATCH v5 2/4] advapi32/tests: Fix typo in manifest constant.

[Eric Pouech]

From: Eric Pouech

(harmless, the actual values are the same).

Signed-off-by: Eric Pouech 
---
 dlls/advapi32/tests/security.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index 0917b144648..90c553902cd 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -3118,7 +3118,7 @@ static void test_process_security_child(void)
     /* Test thread security */
     handle = OpenThread( THREAD_TERMINATE, FALSE, GetCurrentThreadId() );
     ok(handle != NULL, "OpenThread(THREAD_TERMINATE) with err:%ld\n", GetLastError());
-    TEST_GRANTED_ACCESS( handle, PROCESS_TERMINATE );
+    TEST_GRANTED_ACCESS( handle, THREAD_TERMINATE );
     CloseHandle( handle );

handle = OpenThread( THREAD_SET_THREAD_TOKEN, FALSE, GetCurrentThreadId() );

[PATCH v5 3/4] advapi32: Test some other cases of process access rights mapping.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 dlls/advapi32/tests/security.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index 90c553902cd..0fd31585f1c 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -6154,6 +6154,41 @@ static void test_process_access(void)
        "expected PROCESS_QUERY_INFORMATION|PROCESS_QUERY_LIMITED_INFORMATION, got %#lx\n", access);
     CloseHandle(dup);

+    SetLastError( 0xdeadbeef );
+    ret = DuplicateHandle(GetCurrentProcess(), process, GetCurrentProcess(), &dup,
+                          PROCESS_VM_OPERATION, FALSE, 0);
+    ok(ret, "DuplicateHandle error %ld\n", GetLastError());
+    access = get_obj_access(dup);
+    ok(access == PROCESS_VM_OPERATION, "unexpected access right %lx\n", access);
+    CloseHandle(dup);
+
+    SetLastError( 0xdeadbeef );
+    ret = DuplicateHandle(GetCurrentProcess(), process, GetCurrentProcess(), &dup,
+                          PROCESS_VM_WRITE, FALSE, 0);
+    ok(ret, "DuplicateHandle error %ld\n", GetLastError());
+    access = get_obj_access(dup);
+    ok(access == PROCESS_VM_WRITE, "unexpected access right %lx\n", access);
+    CloseHandle(dup);
+
+    SetLastError( 0xdeadbeef );
+    ret = DuplicateHandle(GetCurrentProcess(), process, GetCurrentProcess(), &dup,
+                          PROCESS_VM_OPERATION | PROCESS_VM_WRITE, FALSE, 0);
+    ok(ret, "DuplicateHandle error %ld\n", GetLastError());
+    access = get_obj_access(dup);
+    todo_wine
+    ok(access == (PROCESS_VM_OPERATION | PROCESS_VM_WRITE | PROCESS_QUERY_LIMITED_INFORMATION) ||
+       broken(access == (PROCESS_VM_OPERATION | PROCESS_VM_WRITE)) /* Win8 and before */,
+       "expected PROCESS_VM_OPERATION|PROCESS_VM_WRITE|PROCESS_QUERY_LIMITED_INFORMATION, got %#lx\n", access);
+    CloseHandle(dup);
+
+    SetLastError( 0xdeadbeef );
+    ret = DuplicateHandle(GetCurrentProcess(), process, GetCurrentProcess(), &dup,
+                          PROCESS_VM_OPERATION | PROCESS_VM_READ, FALSE, 0);
+    ok(ret, "DuplicateHandle error %ld\n", GetLastError());
+    access = get_obj_access(dup);
+    ok(access == (PROCESS_VM_OPERATION | PROCESS_VM_READ), "unexpected access right %lx\n", access);
+    CloseHandle(dup);
+
     TerminateProcess(process, 0);
     CloseHandle(process);
 }

[PATCH v5 4/4] server: Amend process rights mapping.

[Eric Pouech]

From: Eric Pouech

PROCESS_VM_OPERATION | PROCESS_VM_WRITE grants
PROCESS_QUERY_LIMITED_INFORMATION rights.

Signed-off-by: Eric Pouech 
---
 dlls/advapi32/tests/security.c | 1 -
 server/process.c               | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index 0fd31585f1c..e215ab9e5f2 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -6175,7 +6175,6 @@ static void test_process_access(void)
                           PROCESS_VM_OPERATION | PROCESS_VM_WRITE, FALSE, 0);
     ok(ret, "DuplicateHandle error %ld\n", GetLastError());
     access = get_obj_access(dup);
-    todo_wine
     ok(access == (PROCESS_VM_OPERATION | PROCESS_VM_WRITE | PROCESS_QUERY_LIMITED_INFORMATION) ||
        broken(access == (PROCESS_VM_OPERATION | PROCESS_VM_WRITE)) /* Win8 and before */,
        "expected PROCESS_VM_OPERATION|PROCESS_VM_WRITE|PROCESS_QUERY_LIMITED_INFORMATION, got %#lx\n", access);
diff --git a/server/process.c b/server/process.c
index 49f5c75005f..f386239919d 100644
--- a/server/process.c
+++ b/server/process.c
@@ -805,6 +805,8 @@ static unsigned int process_map_access( struct object *obj, unsigned int access
     access = default_map_access( obj, access );
     if (access & PROCESS_QUERY_INFORMATION) access |= PROCESS_QUERY_LIMITED_INFORMATION;
     if (access & PROCESS_SET_INFORMATION) access |= PROCESS_SET_LIMITED_INFORMATION;
+    if ((access & (PROCESS_VM_OPERATION | PROCESS_VM_WRITE)) == (PROCESS_VM_OPERATION | PROCESS_VM_WRITE))
+        access |= PROCESS_QUERY_LIMITED_INFORMATION;
     return access;
 }

[PATCH v5 4/4] server: Amend process rights mapping.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149618

Your paranoid android.

=== debian11b (64 bit WoW report) ===

user32:
input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000005E0104, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032

[eric pouech (＠epo)]

On Tue Nov 12 11:45:54 2024 +0000, Dmitry Timoshkov wrote:
> Any real test is better than a pure cliam IMHO.

V5 pushed with add'l tests