diff --git a/channels/sshagent/client/sshagent_main.c b/channels/sshagent/client/sshagent_main.c index 39704eade..855d36a6d 100644 --- a/channels/sshagent/client/sshagent_main.c +++ b/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; } diff --git a/client/X11/xf_utils.c b/client/X11/xf_utils.c index 7355e2587..014ccdd62 100644 --- a/client/X11/xf_utils.c +++ b/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"); - return (env != NULL && strcmp(env, "gnome") == 0); + char* env = winpr_secure_getenv("DESKTOP_SESSION"); + 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, diff --git a/libfreerdp/utils/passphrase.c b/libfreerdp/utils/passphrase.c index 5bc08a0a5..c909ebb60 100644 --- a/libfreerdp/utils/passphrase.c +++ b/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) diff --git a/server/shadow/X11/x11_shadow.c b/server/shadow/X11/x11_shadow.c index 66dd1d1df..de153959c 100644 --- a/server/shadow/X11/x11_shadow.c +++ b/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")) - setenv("DISPLAY", ":0", 1); + char* disp = winpr_secure_getenv("DISPLAY"); + 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")) - setenv("DISPLAY", ":0", 1); + char* disp = winpr_secure_getenv("DISPLAY"); + if (!disp) + winpr_secure_setenv("DISPLAY", ":0", true); + free(disp); display = XOpenDisplay(NULL); diff --git a/winpr/include/winpr/environment.h b/winpr/include/winpr/environment.h index cd5c1e66e..8908ffdf4 100644 --- a/winpr/include/winpr/environment.h +++ b/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 diff --git a/winpr/libwinpr/environment/environment.c b/winpr/libwinpr/environment/environment.c index 84743e061..259237995 100644 --- a/winpr/libwinpr/environment/environment.c +++ b/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 = NULL; - - env = getenv(lpName); + char* env = winpr_secure_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; +} diff --git a/winpr/libwinpr/timezone/TimeZoneIanaAbbrevMap.c b/winpr/libwinpr/timezone/TimeZoneIanaAbbrevMap.c index f0773f444..691bc3ef3 100644 --- a/winpr/libwinpr/timezone/TimeZoneIanaAbbrevMap.c +++ b/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 = NULL; - if (otz) - oldtz = _strdup(otz); - setenv("TZ", tz, 1); + char* oldtz = winpr_secure_getenv("TZ"); + winpr_secure_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); } diff --git a/winpr/libwinpr/timezone/timezone.c b/winpr/libwinpr/timezone/timezone.c index 053f47920..7021387c4 100644 --- a/winpr/libwinpr/timezone/timezone.c +++ b/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)