[winpr,environment] implement threadsafe env

Implement thread save versions of getenv, setenv, putenv, clearenv and
unsetenv

channels/sshagent/client/sshagent_main.c

@@ -50,6 +50,7 @@
 #include <winpr/synch.h>
 #include <winpr/thread.h>
 #include <winpr/stream.h>
+#include <winpr/environment.h>
 
 #include "sshagent_main.h"
 
@@ -67,7 +68,7 @@ typedef struct
 	IWTSVirtualChannelManager* channel_mgr;
 
 	rdpContext* rdpcontext;
-	const char* agent_uds_path;
+	char* agent_uds_path;
 } SSHAGENT_LISTENER_CALLBACK;
 
 typedef struct
@@ -319,7 +320,7 @@ static UINT sshagent_plugin_initialize(IWTSPlugin* pPlugin, IWTSVirtualChannelMa
 	sshagent->listener_callback->iface.OnNewChannelConnection = sshagent_on_new_channel_connection;
 	sshagent->listener_callback->plugin = pPlugin;
 	sshagent->listener_callback->channel_mgr = pChannelMgr;
-	sshagent->listener_callback->agent_uds_path = getenv("SSH_AUTH_SOCK");
+	sshagent->listener_callback->agent_uds_path = winpr_secure_getenv("SSH_AUTH_SOCK");
 
 	if (sshagent->listener_callback->agent_uds_path == NULL)
 	{
@@ -341,6 +342,8 @@ static UINT sshagent_plugin_initialize(IWTSPlugin* pPlugin, IWTSVirtualChannelMa
 static UINT sshagent_plugin_terminated(IWTSPlugin* pPlugin)
 {
 	SSHAGENT_PLUGIN* sshagent = (SSHAGENT_PLUGIN*)pPlugin;
+	if (sshagent && sshagent->listener_callback)
+		free(sshagent->listener_callback->agent_uds_path);
 	free(sshagent);
 	return CHANNEL_RC_OK;
 }

client/X11/xf_utils.c

@@ -22,6 +22,7 @@
 #include <winpr/assert.h>
 #include <winpr/wtypes.h>
 #include <winpr/path.h>
+#include <winpr/environment.h>
 
 #include "xf_utils.h"
 #include "xfreerdp.h"
@@ -172,8 +173,10 @@ int LogDynAndXGetWindowProperty_ex(wLog* log, const char* file, const char* fkt,
 
 BOOL IsGnome(void)
 {
-	char* env = getenv("DESKTOP_SESSION");
+	char* env = winpr_secure_getenv("DESKTOP_SESSION");
-	return (env != NULL && strcmp(env, "gnome") == 0);
+	const BOOL rc = (env != NULL && strcmp(env, "gnome") == 0);
+	free(env);
+	return rc;
 }
 
 BOOL run_action_script(xfContext* xfc, const char* what, const char* arg, fn_action_script_run fkt,

libfreerdp/utils/passphrase.c

@@ -17,6 +17,8 @@
  * limitations under the License.
  */
 
+#include <winpr/environment.h>
+
 #include <freerdp/config.h>
 #include <freerdp/freerdp.h>
 
@@ -252,12 +254,15 @@ static const char* freerdp_passphrase_read_askpass(const char* prompt, char* buf
 const char* freerdp_passphrase_read(rdpContext* context, const char* prompt, char* buf,
                                     size_t bufsiz, int from_stdin)
 {
-	const char* askpass_env = getenv("FREERDP_ASKPASS");
+	char* askpass_env = winpr_secure_getenv("FREERDP_ASKPASS");
 
+	char* rc = NULL;
 	if (askpass_env)
-		return freerdp_passphrase_read_askpass(prompt, buf, bufsiz, askpass_env);
+		rc = freerdp_passphrase_read_askpass(prompt, buf, bufsiz, askpass_env);
 	else
-		return freerdp_passphrase_read_tty(context, prompt, buf, bufsiz, from_stdin);
+		rc = freerdp_passphrase_read_tty(context, prompt, buf, bufsiz, from_stdin);
+	free(askpass_env);
+	return rc;
 }
 
 int freerdp_interruptible_getc(rdpContext* context, FILE* f)

server/shadow/X11/x11_shadow.c

@@ -38,6 +38,7 @@
 #include <winpr/synch.h>
 #include <winpr/image.h>
 #include <winpr/sysinfo.h>
+#include <winpr/environment.h>
 
 #include <freerdp/log.h>
 #include <freerdp/codec/color.h>
@@ -989,8 +990,10 @@ static int x11_shadow_subsystem_base_init(x11ShadowSubsystem* subsystem)
 	if (subsystem->display)
 		return 1; /* initialize once */
 
-	if (!getenv("DISPLAY"))
+	char* disp = winpr_secure_getenv("DISPLAY");
-		setenv("DISPLAY", ":0", 1);
+	if (!disp)
+		winpr_secure_setenv("DISPLAY", ":0", true);
+	free(disp);
 
 	if (!XInitThreads())
 		return -1;
@@ -1185,8 +1188,10 @@ UINT32 x11_shadow_enum_monitors(MONITOR_DEF* monitors, UINT32 maxMonitors)
 	int displayHeight = 0;
 	int numMonitors = 0;
 
-	if (!getenv("DISPLAY"))
+	char* disp = winpr_secure_getenv("DISPLAY");
-		setenv("DISPLAY", ":0", 1);
+	if (!disp)
+		winpr_secure_setenv("DISPLAY", ":0", true);
+	free(disp);
 
 	display = XOpenDisplay(NULL);
 

winpr/include/winpr/environment.h

@@ -22,9 +22,63 @@
 #ifndef WINPR_ENVIRONMENT_H
 #define WINPR_ENVIRONMENT_H
 
+#include <stdbool.h>
+
 #include <winpr/winpr.h>
 #include <winpr/wtypes.h>
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
+	/** @brief thread safe getenv
+	 *
+	 *  @param value The key to get the value for
+	 *
+	 *  @return A copy of the string or \b NULL in case not found.
+	 *  @since version 3.10.0
+	 */
+	WINPR_ATTR_MALLOC(free, 1)
+	WINPR_API char* winpr_secure_getenv(const char* value);
+
+	/** @brief thread safe setenv
+	 * Put a copy of the string \b value for key \b name in the environment.
+	 *
+	 *  @return \b 0 in case of success, a negative number in case of an error
+	 *  @since version 3.10.0
+	 */
+	WINPR_API int winpr_secure_setenv(const char* name, const char* value, bool overwrite);
+
+	/** @brief thread safe putenv
+	 * Put a copy of the string \b value in the environment. The string must have a <key>=<value>
+	 * syntax.
+	 *
+	 *  @return \b 0 in case of success, a negative number in case of an error
+	 *  @since version 3.10.0
+	 */
+	WINPR_API int winpr_secure_putenv(const char* value);
+
+	/** @brief thread safe unsetenv
+	 * Clear a environment variable with key \b name from the environment
+	 *
+	 *  @return \b 0 in case of success, a negative number in case of an error
+	 *  @since version 3.10.0
+	 */
+	WINPR_API int winpr_secure_unsetenv(const char* name);
+
+	/** @brief thread safe clearenv
+	 * Clear all strings from the environment
+	 *
+	 *  @return \b 0 in case of success, a negative number in case of an error
+	 *  @since version 3.10.0
+	 */
+	WINPR_API int winpr_secure_clearenv(void);
+
+#ifdef __cplusplus
+}
+#endif
+
 #ifndef _WIN32
 
 #ifdef __cplusplus

winpr/libwinpr/environment/environment.c

@@ -26,7 +26,7 @@
 #include <winpr/error.h>
 #include <winpr/file.h>
 #include <winpr/string.h>
-
+#include <winpr/collections.h>
 #include <winpr/environment.h>
 
 #ifndef _WIN32
@@ -151,10 +151,7 @@ BOOL NeedCurrentDirectoryForExePathW(LPCWSTR ExeName)
 DWORD GetEnvironmentVariableA(LPCSTR lpName, LPSTR lpBuffer, DWORD nSize)
 {
 #if !defined(_UWP)
-	size_t length = 0;
+	char* env = winpr_secure_getenv(lpName);
-	char* env = NULL;
-
-	env = getenv(lpName);
 
 	if (!env)
 	{
@@ -162,13 +159,17 @@ DWORD GetEnvironmentVariableA(LPCSTR lpName, LPSTR lpBuffer, DWORD nSize)
 		return 0;
 	}
 
-	length = strlen(env);
+	const size_t length = strlen(env);
 
 	if ((length + 1 > nSize) || (!lpBuffer))
+	{
+		free(env);
 		return (DWORD)length + 1;
+	}
 
 	CopyMemory(lpBuffer, env, length);
 	lpBuffer[length] = '\0';
+	free(env);
 
 	return (DWORD)length;
 #else
@@ -191,12 +192,12 @@ BOOL SetEnvironmentVariableA(LPCSTR lpName, LPCSTR lpValue)
 
 	if (lpValue)
 	{
-		if (0 != setenv(lpName, lpValue, 1))
+		if (0 != winpr_secure_setenv(lpName, lpValue, 1))
 			return FALSE;
 	}
 	else
 	{
-		if (0 != unsetenv(lpName))
+		if (0 != winpr_secure_unsetenv(lpName))
 			return FALSE;
 	}
 
@@ -726,3 +727,132 @@ DWORD GetEnvironmentVariableX(const char* lpName, char* lpBuffer, DWORD nSize)
 }
 
 #endif
+
+static INIT_ONCE sEnvGuard = INIT_ONCE_STATIC_INIT;
+static wHashTable* sEnvStrings = NULL;
+
+static void clear_env_strings(void)
+{
+	HashTable_Free(sEnvStrings);
+	sEnvStrings = NULL;
+}
+
+WINPR_ATTR_MALLOC(free, 1)
+static char* split(const char* env, const char** key, const char** value)
+{
+	*key = NULL;
+	*value = NULL;
+	if (!env)
+		return NULL;
+	char* copy = strdup(env);
+	if (!copy)
+		return NULL;
+
+	char* sep = strchr(copy, '=');
+	if (!sep)
+	{
+		free(copy);
+		return NULL;
+	}
+	*key = copy;
+	*value = &sep[1];
+	*sep = '\0';
+
+	return copy;
+}
+
+static BOOL CALLBACK sEnvStringsInitialize(PINIT_ONCE once, PVOID param, PVOID* context)
+{
+	(void)atexit(clear_env_strings);
+	WINPR_ASSERT(!sEnvStrings);
+	sEnvStrings = HashTable_New(TRUE);
+	if (!sEnvStrings)
+		return FALSE;
+	if (!HashTable_SetupForStringData(sEnvStrings, TRUE))
+		return FALSE;
+
+	char** cur = environ;
+	while (cur && (*cur))
+	{
+		const char* key = NULL;
+		const char* value = NULL;
+		char* cp = split(*cur++, &key, &value);
+		if (!cp)
+			continue;
+
+		HashTable_Insert(sEnvStrings, key, value);
+		free(cp);
+	}
+
+	return TRUE;
+}
+
+static void setup(void)
+{
+	InitOnceExecuteOnce(&sEnvGuard, sEnvStringsInitialize, NULL, NULL);
+	HashTable_Lock(sEnvStrings);
+}
+
+char* winpr_secure_getenv(const char* key)
+{
+	setup();
+	char* rc = NULL;
+	const char* value = HashTable_GetItemValue(sEnvStrings, key);
+	if (value)
+		rc = strdup(value);
+	HashTable_Unlock(sEnvStrings);
+	return rc;
+}
+
+int winpr_secure_setenv(const char* name, const char* value, bool overwrite)
+{
+	int rc = -1;
+	setup();
+	if (!overwrite)
+	{
+		if (HashTable_GetItemValue(sEnvStrings, name))
+			rc = 1;
+	}
+	if (rc < 0)
+	{
+		if (!HashTable_Insert(sEnvStrings, name, value))
+			rc = -2;
+		else
+			rc = 0;
+	}
+	HashTable_Unlock(sEnvStrings);
+	return rc;
+}
+
+int winpr_secure_putenv(const char* env)
+{
+	setup();
+
+	int rc = -1;
+	const char* key = NULL;
+	const char* value = NULL;
+	char* cp = split(env, &key, &value);
+	if (cp)
+	{
+		rc = HashTable_Insert(sEnvStrings, key, value) ? 0 : -2;
+	}
+	free(cp);
+	HashTable_Unlock(sEnvStrings);
+	return rc;
+}
+
+int winpr_secure_unsetenv(const char* name)
+{
+	setup();
+	const int rc = HashTable_Remove(sEnvStrings, name) ? 0 : -1;
+	HashTable_Unlock(sEnvStrings);
+	return rc;
+}
+
+int winpr_secure_clearenv(void)
+{
+	setup();
+	HashTable_Clear(sEnvStrings);
+	HashTable_Unlock(sEnvStrings);
+	return 0;
+}

winpr/libwinpr/timezone/TimeZoneIanaAbbrevMap.c

@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include <winpr/environment.h>
 #include <winpr/string.h>
 
 typedef struct
@@ -73,11 +74,8 @@ static void append_timezone(const char* dir, const char* name)
 	if (!tz)
 		return;
 
-	const char* otz = getenv("TZ");
+	char* oldtz = winpr_secure_getenv("TZ");
-	char* oldtz = NULL;
+	winpr_secure_setenv("TZ", tz, 1);
-	if (otz)
-		oldtz = _strdup(otz);
-	setenv("TZ", tz, 1);
 	tzset();
 	const time_t t = time(NULL);
 	struct tm lt = { 0 };
@@ -85,11 +83,11 @@ static void append_timezone(const char* dir, const char* name)
 	append(tz, lt.tm_zone);
 	if (oldtz)
 	{
-		setenv("TZ", oldtz, 1);
+		winpr_secure_setenv("TZ", oldtz, 1);
 		free(oldtz);
 	}
 	else
-		unsetenv("TZ");
+		winpr_secure_unsetenv("TZ");
 	free(tz);
 }
 

winpr/libwinpr/timezone/timezone.c

@@ -875,13 +875,14 @@ DWORD EnumDynamicTimeZoneInformation(const DWORD dwIndex,
 	const time_t t = time(NULL);
 	struct tm tres = { 0 };
 
-	const char* tz = getenv("TZ");
+	char* tz = winpr_secure_getenv("TZ");
 	char* tzcopy = NULL;
 	if (tz)
 	{
 		size_t tzianalen = 0;
 		winpr_asprintf(&tzcopy, &tzianalen, "TZ=%s", tz);
 	}
+	free(tz);
 
 	char* tziana = NULL;
 	{
@@ -889,15 +890,15 @@ DWORD EnumDynamicTimeZoneInformation(const DWORD dwIndex,
 		winpr_asprintf(&tziana, &tzianalen, "TZ=%s", entry->Iana);
 	}
 	if (tziana)
-		putenv(tziana);
+		winpr_secure_putenv(tziana);
 
 	tzset();
 	struct tm* local_time = localtime_r(&t, &tres);
 	free(tziana);
 	if (tzcopy)
-		putenv(tzcopy);
+		winpr_secure_putenv(tzcopy);
 	else
-		unsetenv("TZ");
+		winpr_secure_unsetenv("TZ");
 	free(tzcopy);
 
 	if (local_time)