Skip to content

Commit 417076b

Browse files
matt335672matt335672
committed
Coverity CIDs 468127 468134 468148
These Coverity warnings all relate to the user of g_setenv() where the return result isn't checked. An additional void function g_setenv_log() is provided which logs failures to set environment variables, and returns no status. This is used in all the places where g_setenv_is currently called.
1 parent f2dd3fb commit 417076b

File tree

7 files changed

+55
-31
lines changed

7 files changed

+55
-31
lines changed

‎common/os_calls.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,6 +3589,23 @@ g_setenv(const char *name, const char *value, int rewrite)
35893589
#endif
35903590
}
35913591

3592+
3593+
/*****************************************************************************/
3594+
/* does not work in win32 */
3595+
void
3596+
g_setenv_log(const char *name, const char *value, int rewrite)
3597+
{
3598+
#if defined(_WIN32)
3599+
return 0;
3600+
#else
3601+
if (setenv(name, value, rewrite) != 0)
3602+
{
3603+
LOG(LOG_LEVEL_WARNING, "Unable to set environment variable '%s' [%s]",
3604+
name, g_get_strerror());
3605+
}
3606+
#endif
3607+
}
3608+
35923609
/*****************************************************************************/
35933610
/* does not work in win32 */
35943611
char *

‎common/os_calls.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,17 @@ int g_setpgid(int pid, int pgid);
381381
void g_clearenv(void);
382382
int g_setenv(const char *name, const char *value, int rewrite);
383383
char *g_getenv(const char *name);
384+
/**
385+
* Calls g_setenv(), logging failures
386+
*
387+
* @param name Name to set
388+
* @param value String to set $name to
389+
* @param rewrite Set to non-zero to allow rewriting of existing names
390+
*
391+
* Unlike g_setenv() this function returns no value. Use this function if the
392+
* only reasonable thing to do on failure is to log it.
393+
*/
394+
void g_setenv_log(const char *name, const char *value, int rewrite);
384395
int g_exit(int exit_code);
385396
int g_getpid(void);
386397
int g_sigterm(int pid);

‎sesman/libsesman/verify_user_pam.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -528,12 +528,12 @@ auth_set_env(struct auth_info *auth_info)
528528
for (pam_env = pam_envlist; *pam_env != NULL; ++pam_env)
529529
{
530530
char *str = *pam_env;
531-
int eq_pos = g_pos(str, "=");
531+
char *eq_pos = strchr(str, '=');
532532

533-
if (eq_pos > 0)
533+
if (eq_pos != NULL)
534534
{
535-
str[eq_pos] = '\0';
536-
g_setenv(str, str + eq_pos + 1, 1);
535+
*eq_pos = '\0';
536+
g_setenv_log(str, eq_pos + 1, 1);
537537
}
538538

539539
g_free(str);

‎sesman/libsesman/verify_user_pam_userpass.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,12 +338,12 @@ auth_set_env(struct auth_info *auth_info)
338338
for (pam_env = pam_envlist; *pam_env != NULL; ++pam_env)
339339
{
340340
char *str = *pam_env;
341-
int eq_pos = g_pos(str, "=");
341+
char *eq_pos = strchr(str, '=');
342342

343-
if (eq_pos > 0)
343+
if (eq_pos != NULL)
344344
{
345-
str[eq_pos] = '\0';
346-
g_setenv(str, str + eq_pos + 1, 1);
345+
*eq_pos = '\0';
346+
g_setenv_log(str, eq_pos + 1, 1);
347347
}
348348

349349
g_free(str);

‎sesman/sesexec/env.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,38 +144,38 @@ env_set_user(int uid, char **passwd_file, int display,
144144

145145
if (error == 0)
146146
{
147-
g_setenv("PATH", "/sbin:/bin:/usr/bin:/usr/local/bin", 1);
147+
g_setenv_log("PATH", "/sbin:/bin:/usr/bin:/usr/local/bin", 1);
148148
}
149149
#endif
150150
if (error == 0)
151151
{
152-
g_setenv("SHELL", pw_shell, 1);
153-
g_setenv("USER", pw_username, 1);
154-
g_setenv("LOGNAME", pw_username, 1);
152+
g_setenv_log("SHELL", pw_shell, 1);
153+
g_setenv_log("USER", pw_username, 1);
154+
g_setenv_log("LOGNAME", pw_username, 1);
155155
g_snprintf(text, sizeof(text), "%d", uid);
156-
g_setenv("UID", text, 1);
157-
g_setenv("HOME", pw_dir, 1);
156+
g_setenv_log("UID", text, 1);
157+
g_setenv_log("HOME", pw_dir, 1);
158158
g_set_current_dir(pw_dir);
159159
g_snprintf(text, sizeof(text), ":%d.0", display);
160-
g_setenv("DISPLAY", text, 1);
160+
g_setenv_log("DISPLAY", text, 1);
161161
// Use our PID as the XRDP_SESSION value
162162
g_snprintf(text, sizeof(text), "%d", g_pid);
163-
g_setenv("XRDP_SESSION", text, 1);
163+
g_setenv_log("XRDP_SESSION", text, 1);
164164
/* XRDP_SOCKET_PATH should be set here. It's used by
165165
* xorgxrdp and the pulseaudio plugin */
166166
g_snprintf(text, sizeof(text), XRDP_SOCKET_PATH, uid);
167-
g_setenv("XRDP_SOCKET_PATH", text, 1);
167+
g_setenv_log("XRDP_SOCKET_PATH", text, 1);
168168
/* pulse sink socket */
169169
g_snprintf(text, sizeof(text), CHANSRV_PORT_OUT_BASE_STR, display);
170-
g_setenv("XRDP_PULSE_SINK_SOCKET", text, 1);
170+
g_setenv_log("XRDP_PULSE_SINK_SOCKET", text, 1);
171171
/* pulse source socket */
172172
g_snprintf(text, sizeof(text), CHANSRV_PORT_IN_BASE_STR, display);
173-
g_setenv("XRDP_PULSE_SOURCE_SOCKET", text, 1);
173+
g_setenv_log("XRDP_PULSE_SOURCE_SOCKET", text, 1);
174174
if (g_cfg->sec.xauth_in_sysdir)
175175
{
176176
g_snprintf(text, sizeof(text), XRDP_SOCKET_PATH "/Xauthority",
177177
uid);
178-
g_setenv("XAUTHORITY", text, 1);
178+
g_setenv_log("XAUTHORITY", text, 1);
179179
}
180180
if ((env_names != 0) && (env_values != 0) &&
181181
(env_names->count == env_values->count))
@@ -184,7 +184,7 @@ env_set_user(int uid, char **passwd_file, int display,
184184
{
185185
name = (char *) list_get_item(env_names, index),
186186
value = (char *) list_get_item(env_values, index),
187-
g_setenv(name, value, 1);
187+
g_setenv_log(name, value, 1);
188188
}
189189
}
190190
g_gethostname(hostname, 255);

‎sesman/sesexec/session.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,19 +357,19 @@ prepare_xorg_xserver_params(const struct session_parameters *s,
357357

358358
/* some args are passed via env vars */
359359
g_snprintf(text, sizeof(text), "%d", s->width);
360-
g_setenv("XRDP_START_WIDTH", text, 1);
360+
g_setenv_log("XRDP_START_WIDTH", text, 1);
361361

362362
g_snprintf(text, sizeof(text), "%d", s->height);
363-
g_setenv("XRDP_START_HEIGHT", text, 1);
363+
g_setenv_log("XRDP_START_HEIGHT", text, 1);
364364

365365
g_snprintf(text, sizeof(text), "%d", g_cfg->sess.max_idle_time);
366-
g_setenv("XRDP_SESMAN_MAX_IDLE_TIME", text, 1);
366+
g_setenv_log("XRDP_SESMAN_MAX_IDLE_TIME", text, 1);
367367

368368
g_snprintf(text, sizeof(text), "%d", g_cfg->sess.max_disc_time);
369-
g_setenv("XRDP_SESMAN_MAX_DISC_TIME", text, 1);
369+
g_setenv_log("XRDP_SESMAN_MAX_DISC_TIME", text, 1);
370370

371371
g_snprintf(text, sizeof(text), "%d", g_cfg->sess.kill_disconnected);
372-
g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1);
372+
g_setenv_log("XRDP_SESMAN_KILL_DISCONNECTED", text, 1);
373373

374374
/* get path of Xorg from config */
375375
xserver = (const char *)list_get_item(g_cfg->xorg_params, 0);

‎sesman/sesexec_control.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,7 @@ sesexec_start(struct pre_session_item *psi)
165165
* in the environment */
166166
char buff[64];
167167
g_snprintf(buff, sizeof(buff), "%d", sck[1]);
168-
if (g_setenv("EICP_FD", buff, 1) < 0)
169-
{
170-
LOG(LOG_LEVEL_ERROR, "Can't set EICP_FD [%s]",
171-
g_get_strerror());
172-
}
168+
g_setenv_log("EICP_FD", buff, 1);
173169

174170
/* [Development] Log all file descriptors not marked cloexec
175171
* other than stdin, stdout, stderr, and the EICP fd in sck[1].

0 commit comments

Comments
 (0)