[PATCH v2 0/2] MR7579: ntdll: Return STATUS_ACCESS_VIOLATION if ret_len is not writable

[Dylan Donnell (＠dy-tea)]

Required for implementation correctness check in certain protection software.

Thanks to mkrsym1  for the patch!

--
  v2: ntdll: Write ret_len before checking length in ThreadHideFromDebugger

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

[PATCH v2 1/2] ntdll: Return STATUS_ACCESS_VIOLATION if ret_len is not writable

[Dylan Donnell]

From: Dylan Donnell

---
 dlls/ntdll/unix/thread.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c
index b64a7dd40af..eb94b21032f 100644
--- a/dlls/ntdll/unix/thread.c
+++ b/dlls/ntdll/unix/thread.c
@@ -2040,9 +2040,23 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE handle, THREADINFOCLASS class,
                                           void *data, ULONG length, ULONG *ret_len )
 {
     unsigned int status;
+    MEMORY_BASIC_INFORMATION memory_info;

TRACE("(%p,%d,%p,%x,%p)\n", handle, class, data, (int)length, ret_len);

+    if (ret_len)
+    {
+        /* check whether ret_len is writable */
+        if (NtQueryVirtualMemory( GetCurrentProcess(), ret_len, MemoryBasicInformation, &memory_info, sizeof(memory_info), NULL ) != STATUS_SUCCESS) 
+        {
+            return STATUS_ACCESS_VIOLATION;
+        }
+        if (!(memory_info.Protect & (PAGE_READWRITE | PAGE_WRITECOPY | PAGE_EXECUTE_READWRITE | PAGE_EXECUTE_WRITECOPY))) 
+        {
+            return STATUS_ACCESS_VIOLATION;
+        }
+    }
+
     switch (class)
     {
     case ThreadBasicInformation:

[PATCH v2 2/2] ntdll: Write ret_len before checking length in ThreadHideFromDebugger

[Dylan Donnell]

From: Dylan Donnell

---
 dlls/ntdll/unix/thread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/ntdll/unix/thread.c b/dlls/ntdll/unix/thread.c
index eb94b21032f..24c7162ea03 100644
--- a/dlls/ntdll/unix/thread.c
+++ b/dlls/ntdll/unix/thread.c
@@ -2289,6 +2289,7 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE handle, THREADINFOCLASS class,
         return get_thread_wow64_context( handle, data, length );

case ThreadHideFromDebugger:
+        if (ret_len) *ret_len = sizeof(BOOLEAN);
         if (length != sizeof(BOOLEAN)) return STATUS_INFO_LENGTH_MISMATCH;
         if (!data) return STATUS_ACCESS_VIOLATION;
         SERVER_START_REQ( get_thread_info )
@@ -2299,7 +2300,6 @@ NTSTATUS WINAPI NtQueryInformationThread( HANDLE handle, THREADINFOCLASS class,
             *(BOOLEAN*)data = !!(reply->flags & GET_THREAD_INFO_FLAG_DBG_HIDDEN);
         }
         SERVER_END_REQ;
-        if (ret_len) *ret_len = sizeof(BOOLEAN);
         return STATUS_SUCCESS;

case ThreadPriorityBoost:

[Dylan Donnell (＠dy-tea)]

Where is the best place to add the test?

[Paul Gofman (＠gofman)]

On Fri Mar 14 18:16:05 2025 +0000, Dylan Donnell wrote:
> Where is the best place to add the test?

To dlls/ntdll/tests/info.c:test_HideFromDebugger(). Currently NtQueryInformationThread is always tested there without ret_len parameter. I'd add tests to check what it returns in cases when length is wrong, when length is correct but handle is wrong and when everything is correct (and setting the length variable to be returned to 0xdeadbeef before each test to make sure if that is actually changed or not).