Move get_tmp_dir to win32-util.c and error out on failure

Currently we only warn in get_tmp_dir fails and set o->tmp_dir to a null pointer. This will not be caught by check_file_access_chroot either since that ignores NULL pointers but other parts of OpenVPN will assume that tmp_dir is set to a non-NULL string. Also move get_tmp_dir to win32-util.c to use it in unit tests. Change-Id: I525ccf7872880367b248ebebb0ddc83551498042 Signed-off-by: Arne Schwabe <[email protected]> Acked-by: Frank Lichtenheld <[email protected]> Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg27964.html Signed-off-by: Gert Doering <[email protected]>
OpenVPN · Jan 8, 2024 · f0a17ed · f0a17ed
1 parent 7268e14
commit f0a17ed
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 32 deletions.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
@@ -885,7 +885,15 @@ init_options(struct options *o, const bool init_gc)
 #ifdef _WIN32
     /* On Windows, find temp dir via environment variables */
     o->tmp_dir = win_get_tempdir();
-#else
+
+    if (!o->tmp_dir)
+    {
+        /* Error out if we can't find a valid temporary directory, which should
+         * be very unlikely. */
+        msg(M_USAGE, "Could not find a suitable temporary directory."
+            " (GetTempPath() failed).  Consider using --tmp-dir");
+    }
+#else  /* ifdef _WIN32 */
     /* Non-windows platforms use $TMPDIR, and if not set, default to '/tmp' */
     o->tmp_dir = getenv("TMPDIR");
     if (!o->tmp_dir)

diff --git a/src/openvpn/win32-util.c b/src/openvpn/win32-util.c
@@ -147,4 +147,26 @@ win_safe_filename(const char *fn)
     }
     return true;
 }
+
+const char *
+win_get_tempdir(void)
+{
+    static char tmpdir[MAX_PATH];
+    WCHAR wtmpdir[MAX_PATH];
+
+    if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
+    {
+        return NULL;
+    }
+
+    if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > sizeof(tmpdir))
+    {
+        msg(M_WARN, "Could not get temporary directory. Path is too long."
+            "  Consider using --tmp-dir");
+        return NULL;
+    }
+
+    WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, NULL);
+    return tmpdir;
+}
 #endif /* _WIN32 */
diff --git a/src/openvpn/win32-util.h b/src/openvpn/win32-util.h
@@ -40,5 +40,8 @@ char *utf16to8(const wchar_t *utf16, struct gc_arena *gc);
 /* return true if filename is safe to be used on Windows */
 bool win_safe_filename(const char *fn);
 
+/* Find temporary directory */
+const char *win_get_tempdir(void);
+
 #endif /* OPENVPN_WIN32_UTIL_H */
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
@@ -1137,34 +1137,6 @@ set_win_sys_path_via_env(struct env_set *es)
     set_win_sys_path(buf, es);
 }
 
-
-const char *
-win_get_tempdir(void)
-{
-    static char tmpdir[MAX_PATH];
-    WCHAR wtmpdir[MAX_PATH];
-
-    if (!GetTempPathW(_countof(wtmpdir), wtmpdir))
-    {
-        /* Warn if we can't find a valid temporary directory, which should
-         * be unlikely.
-         */
-        msg(M_WARN, "Could not find a suitable temporary directory."
-            " (GetTempPath() failed).  Consider using --tmp-dir");
-        return NULL;
-    }
-
-    if (WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, NULL, 0, NULL, NULL) > sizeof(tmpdir))
-    {
-        msg(M_WARN, "Could not get temporary directory. Path is too long."
-            "  Consider using --tmp-dir");
-        return NULL;
-    }
-
-    WideCharToMultiByte(CP_UTF8, 0, wtmpdir, -1, tmpdir, sizeof(tmpdir), NULL, NULL);
-    return tmpdir;
-}
-
 static bool
 win_block_dns_service(bool add, int index, const HANDLE pipe)
 {

diff --git a/src/openvpn/win32.h b/src/openvpn/win32.h
@@ -286,9 +286,6 @@ char *get_win_sys_path(void);
 /* call self in a subprocess */
 void fork_to_self(const char *cmdline);
 
-/* Find temporary directory */
-const char *win_get_tempdir(void);
-
 bool win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel);
 
 bool win_wfp_uninit(const NET_IFINDEX index, const HANDLE msg_channel);