[PATCH v4] msvcrt: Don't crash if _vsnwprintf gets NULL as format string and add test

[Fabian Maurer]

v2
Fix crash on older test machines
v3
Check for dll function instead of OS version
Add test for errno
Moved test to msvcrt90 to  be able to use _set_invalid_parameter_handler
v4
proper patch name

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
---
 dlls/msvcr90/tests/msvcr90.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 dlls/msvcrt/printf.h         |  7 +++++++
 2 files changed, 50 insertions(+)

diff --git a/dlls/msvcr90/tests/msvcr90.c b/dlls/msvcr90/tests/msvcr90.c
index 0975d00331..851c9c0a23 100644
--- a/dlls/msvcr90/tests/msvcr90.c
+++ b/dlls/msvcr90/tests/msvcr90.c
@@ -133,6 +133,7 @@ static char* (__cdecl *p_setlocale)(int, const char*);
 static int (__cdecl *p__fpieee_flt)(ULONG, EXCEPTION_POINTERS*, int (__cdecl *handler)(_FPIEEE_RECORD*));
 static int (__cdecl *p__memicmp)(const char*, const char*, size_t);
 static int (__cdecl *p__memicmp_l)(const char*, const char*, size_t, _locale_t);
+static int (__cdecl *p__vsnwprintf)(wchar_t *buffer,size_t count, const wchar_t *format, va_list valist);
 
 /* make sure we use the correct errno */
 #undef errno
@@ -401,6 +402,7 @@ static BOOL init(void)
     SET(p__fpieee_flt, "_fpieee_flt");
     SET(p__memicmp, "_memicmp");
     SET(p__memicmp_l, "_memicmp_l");
+    SET(p__vsnwprintf, "_vsnwprintf");
 
     if (sizeof(void *) == 8)
     {
@@ -1864,6 +1866,46 @@ static void test__memicmp_l(void)
     ok(ret == -1, "got %d\n", ret);
 }
 
+static int WINAPIV _vsnwprintf_wrapper(wchar_t *str, size_t len, const wchar_t *format, ...)
+{
+    int ret;
+    __ms_va_list valist;
+    __ms_va_start(valist, format);
+    ret = p__vsnwprintf(str, len, format, valist);
+    __ms_va_end(valist);
+    return ret;
+}
+
+static BOOL is_vista_or_newer(void)
+{
+    return GetProcAddress(GetModuleHandleA("msvcrt.dll"), "_vsnwprintf_s") != NULL;
+}
+
+void test__vsnwprintf(void)
+{
+    int ret;
+    WCHAR str[2] = {0};
+
+    /* Test with format string set to NULL, we should not crash on vista or newer. */
+    if(is_vista_or_newer())
+    {
+        _invalid_parameter_handler old_handler = p_set_invalid_parameter_handler(test_invalid_parameter_handler);
+
+        SET_EXPECT(invalid_parameter_handler);
+        errno = 0xdeadbeef;
+        str[0] = 'x';
+        ret = _vsnwprintf_wrapper(str, 0, NULL);
+        ok(ret == -1, "got %d, expected -1\n", ret);
+        ok(str[0] == 'x', "Expected string to be unchanged.\n");
+        CHECK_CALLED(invalid_parameter_handler, EINVAL);
+
+        ok(p_set_invalid_parameter_handler(old_handler) == test_invalid_parameter_handler,
+                "Cannot reset invalid parameter handler\n");
+    }
+    else
+        win_skip("_vsnwprintf would crash with a NULL argument.\n");
+}
+
 START_TEST(msvcr90)
 {
     if(!init())
@@ -1897,6 +1939,7 @@ START_TEST(msvcr90)
     test__mbstok_s();
     test__memicmp();
     test__memicmp_l();
+    test__vsnwprintf();
 #ifdef __i386__
     test__fpieee_flt();
 #endif
diff --git a/dlls/msvcrt/printf.h b/dlls/msvcrt/printf.h
index 4ba02bafdb..def5342252 100644
--- a/dlls/msvcrt/printf.h
+++ b/dlls/msvcrt/printf.h
@@ -376,6 +376,13 @@ int FUNC_NAME(pf_printf)(FUNC_NAME(puts_clbk) pf_puts, void *puts_ctx, const API
 
     TRACE("Format is: %s\n", FUNC_NAME(debugstr)(fmt));
 
+    if(!fmt)
+    {
+        *MSVCRT__errno() = MSVCRT_EINVAL;
+        MSVCRT__invalid_parameter(NULL, NULL, NULL, 0, 0);
+        return -1;
+    }
+
     if(!locale)
         locinfo = get_locinfo();
     else
-- 
2.15.0

[Piotr Caban]

Hi Fabian,

On 11/25/17 18:55, Fabian Maurer wrote:
> +void test__vsnwprintf(void)
> +{
> +    int ret;
> +    WCHAR str[2] = {0};
> +
> +    /* Test with format string set to NULL, we should not crash on vista or newer. */
> +    if(is_vista_or_newer())
> Do you need this condition? I'm expecting msvcr90 behavior to be 
> consistent between OS version.
> +    if(!fmt)
> +    {
> +        *MSVCRT__errno() = MSVCRT_EINVAL;
> +        MSVCRT__invalid_parameter(NULL, NULL, NULL, 0, 0);
> +        return -1;
> +    }
> Please use MSVCRT_CHECK_PMT macro here.

Thanks,
Piotr

[Fabian Maurer]

> Do you need this condition? I'm expecting msvcr90 behavior to be
> consistent between OS version.

Indeed, thank you. This makes it simpler.
> Please use MSVCRT_CHECK_PMT macro here.

Done, sent in an updated patch.

Regards,
Fabian Maurer