[PATCH 0/3] MR187: ntdll/tests: Test if RtlFormatMessage modifies va_list on error.

[Daniel Lehman (＠dlehman25)]

Signed-off-by: Daniel Lehman

[PATCH 1/3] ntdll/tests: Test if RtlFormatMessage modifies va_list on error.

[Daniel Lehman]

From: Daniel Lehman

Signed-off-by: Daniel Lehman 
---
 dlls/ntdll/tests/rtlstr.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/dlls/ntdll/tests/rtlstr.c b/dlls/ntdll/tests/rtlstr.c
index 1bce812ff13..5ffa16f8e16 100644
--- a/dlls/ntdll/tests/rtlstr.c
+++ b/dlls/ntdll/tests/rtlstr.c
@@ -2641,6 +2641,25 @@ static void WINAPIV testfmt( const WCHAR *src, const WCHAR *expect, ULONG width,
     ok( size == (lstrlenW(expect) + 1) * sizeof(WCHAR), "%s: wrong size %lu\n", debugstr_w(src), size );
 }

+static void testfmt_arg_eaten( const WCHAR *src, ... )
+{
+    va_list args;
+    NTSTATUS status;
+    WCHAR *arg, buffer[1];
+    ULONG size = 0xdeadbeef;
+
+    buffer[0] = 0xcccc;
+    va_start( args, src );
+    status = pRtlFormatMessage( src, 0, FALSE, FALSE, FALSE, &args, buffer, ARRAY_SIZE(buffer), &size );
+    ok( status == STATUS_BUFFER_OVERFLOW, "%s: failed %lx\n", debugstr_w(src), status );
+    todo_wine
+    ok( buffer[0] == 0xcccc, "%s: got %x\n", debugstr_w(src), buffer[0] );
+    ok( size == 0xdeadbeef, "%s: wrong size %lu\n", debugstr_w(src), size );
+    arg = va_arg( args, WCHAR * );
+    ok( !wcscmp( L"unused", arg ), "%s: wrong arg %s\n", debugstr_w(src), debugstr_w(arg) );
+    va_end( args );
+}
+
 static void test_RtlFormatMessage(void)
 {
     WCHAR buffer[128];
@@ -2888,6 +2907,8 @@ static void test_RtlFormatMessage(void)
     ok( !lstrcmpW( buffer, L"abcxxxxxxx" ), "got %s\n", wine_dbgstr_w(buffer) );
     ok( size == 0xdeadbeef, "wrong size %lu\n", size );

+    /* va_arg is eaten even in case of buffer overflow */
+    testfmt_arg_eaten( L"%1!s! %2!s!", L"eaten", L"unused" );
 }

START_TEST(rtlstr)

[PATCH 2/3] kernel32/tests: Test if FormatMessage modifies va_list on error.

[Daniel Lehman]

From: Daniel Lehman

Signed-off-by: Daniel Lehman 
---
 dlls/kernel32/tests/format_msg.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/dlls/kernel32/tests/format_msg.c b/dlls/kernel32/tests/format_msg.c
index 2d317d1bb9e..e2a4f05ffdb 100644
--- a/dlls/kernel32/tests/format_msg.c
+++ b/dlls/kernel32/tests/format_msg.c
@@ -1005,6 +1005,25 @@ static void test_message_wrap(void)
     ok(!strcmp("short\r\nlong?line", out),"failed out=[%s]\n",out);
 }

+static void test_message_arg_eaten( const WCHAR *src, ... )
+{
+    DWORD ret;
+    va_list list;
+    WCHAR *arg, out[1];
+
+    out[0] = 0xcccc;
+    va_start(list, src);
+    SetLastError(0xdeadbeef);
+    ret = FormatMessageW(FORMAT_MESSAGE_FROM_STRING, src, 0, 0, out, ARRAY_SIZE(out), &list);
+    ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER,
+       "Expected GetLastError() to return ERROR_INSUFFICIENT_BUFFER, got %lu\n", GetLastError());
+    ok(ret == 0, "Expected FormatMessageW to return 0, got %lu\n", ret);
+    ok(out[0] == 0, "Expected null, got %ls\n", out);
+    arg = va_arg( list, WCHAR * );
+    ok(!wcscmp( L"unused", arg ), "Expected 'unused', got %s\n", wine_dbgstr_w(arg));
+    va_end(list);
+}
+
 static void test_message_insufficient_buffer(void)
 {
     static const char init_buf[] = {'x', 'x', 'x', 'x', 'x'};
@@ -1153,6 +1172,9 @@ static void test_message_insufficient_buffer_wide(void)
         HeapFree( GetProcessHeap(), 0, tmp );
         HeapFree( GetProcessHeap(), 0, buf );
     }
+
+    /* va_arg is eaten even in case of insufficient buffer */
+    test_message_arg_eaten( L"%1!s! %2!s!", L"eaten", L"unused" );
 }

static void test_message_null_buffer(void)

[PATCH 3/3] kernelbase: Pass va_list copy to internal RtlFormatMessage.

[Daniel Lehman]

From: Daniel Lehman

va_list passed to RtlFormatMessage is modified even on error
in this case, if the buffer is not large enough, STATUS_BUFFER_OVERFLOW
is returned and FormatMessage tries again, but the va_list pointer is
now moved to a later argument, so the next call reads off the end, crashing

Signed-off-by: Daniel Lehman 
---
 dlls/kernelbase/locale.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/dlls/kernelbase/locale.c b/dlls/kernelbase/locale.c
index ac4dc188345..d4887722209 100644
--- a/dlls/kernelbase/locale.c
+++ b/dlls/kernelbase/locale.c
@@ -5352,6 +5352,7 @@ DWORD WINAPI DECLSPEC_HOTPATCH FormatMessageW( DWORD flags, const void *source,
     if (flags & FORMAT_MESSAGE_ALLOCATE_BUFFER)
     {
         WCHAR *result;
+        va_list args_copy;
         ULONG alloc = max( size * sizeof(WCHAR), 65536 );

for (;;)
@@ -5361,9 +5362,17 @@ DWORD WINAPI DECLSPEC_HOTPATCH FormatMessageW( DWORD flags, const void *source,
                 status = STATUS_NO_MEMORY;
                 break;
             }
-            status = RtlFormatMessage( src, width, !!(flags & FORMAT_MESSAGE_IGNORE_INSERTS),
-                                       FALSE, !!(flags & FORMAT_MESSAGE_ARGUMENT_ARRAY), args,
-                                       result, alloc, &retsize );
+            if (args && !(flags & FORMAT_MESSAGE_ARGUMENT_ARRAY))
+            {
+                va_copy( args_copy, *args );
+                status = RtlFormatMessage( src, width, !!(flags & FORMAT_MESSAGE_IGNORE_INSERTS),
+                                           FALSE, FALSE, &args_copy, result, alloc, &retsize );
+                va_end( args_copy );
+            }
+            else
+                status = RtlFormatMessage( src, width, !!(flags & FORMAT_MESSAGE_IGNORE_INSERTS),
+                                           FALSE, TRUE, args, result, alloc, &retsize );
+
             if (!status)
             {
                 if (retsize <= sizeof(WCHAR)) HeapFree( GetProcessHeap(), 0, result );