Piotr Caban : msvcrt: Improve error handling in getenv_s.

[Alexandre Julliard]

Module: wine
Branch: stable
Commit: 07d01fdeba65d032a9d6dc95199528a690264d6f
URL:    https://gitlab.winehq.org/wine/wine/-/commit/07d01fdeba65d032a9d6dc95199528a...

Author: Piotr Caban 
Date:   Wed Feb 15 13:35:14 2023 +0100

msvcrt: Improve error handling in getenv_s.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54486
(cherry picked from commit b7a586771b96cd38e4afb727fefcab3a9deef3c5)

---

dlls/msvcrt/environ.c       | 41 ++++++++++++++++++++++-------------------
 dlls/msvcrt/tests/environ.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/dlls/msvcrt/environ.c b/dlls/msvcrt/environ.c
index a2626a85c30..39a2f6b4e09 100644
--- a/dlls/msvcrt/environ.c
+++ b/dlls/msvcrt/environ.c
@@ -25,15 +25,12 @@

WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);

-/*********************************************************************
- *              getenv (MSVCRT.@)
- */
-char * CDECL getenv(const char *name)
+static char * getenv_helper(const char *name)
 {
     char **env;
     size_t len;

-    if (!MSVCRT_CHECK_PMT(name != NULL)) return NULL;
+    if (!name) return NULL;
     len = strlen(name);

for (env = MSVCRT__environ; *env; env++)
@@ -49,6 +46,16 @@ char * CDECL getenv(const char *name)
     return NULL;
 }

+/*********************************************************************
+ *              getenv (MSVCRT.@)
+ */
+char * CDECL getenv(const char *name)
+{
+    if (!MSVCRT_CHECK_PMT(name != NULL)) return NULL;
+
+    return getenv_helper(name);
+}
+
 /*********************************************************************
  *              _wgetenv (MSVCRT.@)
  */
@@ -278,24 +285,20 @@ int CDECL _wdupenv_s(wchar_t **buffer, size_t *numberOfElements,
 /******************************************************************
  *		getenv_s (MSVCRT.@)
  */
-int CDECL getenv_s(size_t *pReturnValue, char* buffer, size_t numberOfElements, const char *varname)
+int CDECL getenv_s(size_t *ret_len, char* buffer, size_t len, const char *varname)
 {
     char *e;

-    if (!MSVCRT_CHECK_PMT(pReturnValue != NULL)) return EINVAL;
-    if (!MSVCRT_CHECK_PMT(!(buffer == NULL && numberOfElements > 0))) return EINVAL;
-    if (!MSVCRT_CHECK_PMT(varname != NULL)) return EINVAL;
+    if (!MSVCRT_CHECK_PMT(ret_len != NULL)) return EINVAL;
+    *ret_len = 0;
+    if (!MSVCRT_CHECK_PMT((buffer && len > 0) || (!buffer && !len))) return EINVAL;
+    if (buffer) buffer[0] = 0;
+
+    if (!(e = getenv_helper(varname))) return 0;
+    *ret_len = strlen(e) + 1;
+    if (!len) return 0;
+    if (len < *ret_len) return ERANGE;

-    if (!(e = getenv(varname)))
-    {
-        *pReturnValue = 0;
-        return *_errno() = EINVAL;
-    }
-    *pReturnValue = strlen(e) + 1;
-    if (numberOfElements < *pReturnValue)
-    {
-        return *_errno() = ERANGE;
-    }
     strcpy(buffer, e);
     return 0;
 }
diff --git a/dlls/msvcrt/tests/environ.c b/dlls/msvcrt/tests/environ.c
index 03129dcbf14..3d6b55dffa8 100644
--- a/dlls/msvcrt/tests/environ.c
+++ b/dlls/msvcrt/tests/environ.c
@@ -50,6 +50,7 @@ static void (__cdecl *p_get_environ)(char ***);
 static void (__cdecl *p_get_wenviron)(WCHAR ***);
 static errno_t (__cdecl *p_putenv_s)(const char*, const char*);
 static errno_t (__cdecl *p_wputenv_s)(const wchar_t*, const wchar_t*);
+static errno_t (__cdecl *p_getenv_s)(size_t*, char*, size_t, const char*);

static char ***p_environ;
 static WCHAR ***p_wenviron;
@@ -66,6 +67,7 @@ static void init(void)
     p_get_wenviron = (void *)GetProcAddress(hmod, "_get_wenviron");
     p_putenv_s = (void *)GetProcAddress(hmod, "_putenv_s");
     p_wputenv_s = (void *)GetProcAddress(hmod, "_wputenv_s");
+    p_getenv_s = (void *)GetProcAddress(hmod, "getenv_s");
 }

static void test_system(void)
@@ -242,11 +244,20 @@ static void test__wenviron(void)

static void test_environment_manipulation(void)
 {
+    char buf[256];
     errno_t ret;
+    size_t len;

ok( _putenv("cat=") == 0, "_putenv failed on deletion of nonexistent environment variable\n" );
     ok( _putenv("cat=dog") == 0, "failed setting cat=dog\n" );
     ok( strcmp(getenv("cat"), "dog") == 0, "getenv did not return 'dog'\n" );
+    if (p_getenv_s)
+    {
+        ret = p_getenv_s(&len, buf, sizeof(buf), "cat");
+        ok( !ret, "getenv_s returned %d\n", ret );
+        ok( len == 4, "getenv_s returned length is %Id\n", len);
+        ok( !strcmp(buf, "dog"), "getenv_s did not return 'dog'\n");
+    }
     ok( _putenv("cat=") == 0, "failed deleting cat\n" );

ok( _putenv("=") == -1, "should not accept '=' as input\n" );
@@ -282,6 +293,27 @@ static void test_environment_manipulation(void)
         ret = p_wputenv_s(L"cat", L"");
         ok( !ret, "_wputenv_s returned %d\n", ret);
     }
+
+    if (p_getenv_s)
+    {
+        buf[0] = 'x';
+        len = 1;
+        errno = 0xdeadbeef;
+        ret = p_getenv_s(&len, buf, sizeof(buf), "nonexistent");
+        ok( !ret, "_getenv_s returned %d\n", ret);
+        ok( !len, "getenv_s returned length is %Id\n", len);
+        ok( !buf[0], "buf = %s\n", buf);
+        ok( errno == 0xdeadbeef, "errno = %d\n", errno);
+
+        buf[0] = 'x';
+        len = 1;
+        errno = 0xdeadbeef;
+        ret = p_getenv_s(&len, buf, sizeof(buf), NULL);
+        ok( !ret, "_getenv_s returned %d\n", ret);
+        ok( !len, "getenv_s returned length is %Id\n", len);
+        ok( !buf[0], "buf = %s\n", buf);
+        ok( errno == 0xdeadbeef, "errno = %d\n", errno);
+    }
 }

START_TEST(environ)