Skip to content

Commit 44acc46

Browse files
matt335672matt335672
authored
Merge pull request neutrinolabs#3505 from matt335672/coverity_fixes
Coverity fixes
2 parents fc30a5f + d30f5fe commit 44acc46

File tree

12 files changed

+97
-57
lines changed

12 files changed

+97
-57
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);

‎common/scancode.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -367,19 +367,15 @@ scancode_to_index(unsigned short scancode)
367367
unsigned short
368368
scancode_from_index(int index)
369369
{
370-
index &= 0xff;
371-
unsigned short result;
372-
if (index == SCANCODE_INDEX_PAUSE_KEY)
370+
unsigned short result = index & 0xff;
371+
372+
if (result == SCANCODE_INDEX_PAUSE_KEY)
373373
{
374374
result = SCANCODE_PAUSE_KEY;
375375
}
376-
else if (index < 0x80)
377-
{
378-
result = index;
379-
}
380-
else
376+
else if ((result & 0x80) != 0)
381377
{
382-
result = (index & 0x7f) | 0x100;
378+
result ^= 0x180; // Clear bit 7, set bit 8
383379
}
384380
return result;
385381
}

‎libxrdp/xrdp_iso.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ protocol_mask_to_str(int protocol, char *buff, int bufflen)
6868
/* String is empty */
6969
rlen = g_snprintf(buff, bufflen, "RDP");
7070
}
71-
else if (rlen < bufflen)
71+
else if (rlen > 0 && rlen < bufflen)
7272
{
7373
rlen += g_snprintf(buff + rlen, bufflen - rlen, "%cRDP", delim);
7474
}

‎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].

‎xrdp/xrdp_font.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,13 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
287287
in_uint8s(s, 6);
288288
datasize = FONT_DATASIZE(f);
289289

290-
if (datasize < 0 || datasize > 512)
290+
if (datasize > 512)
291291
{
292292
/* shouldn't happen */
293293
LOG(LOG_LEVEL_ERROR,
294294
"xrdp_font_create: "
295295
"datasize for U+%x wrong "
296-
"width %d, height %d, datasize %d",
296+
"width %d, height %d, datasize %u",
297297
char_count, f->width, f->height, datasize);
298298
break;
299299
}

‎xrdp/xrdp_mm.c

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2751,7 +2751,7 @@ xrdp_mm_get_sesman_port(char *port, int port_bytes)
27512751

27522752
g_memset(cfg_file, 0, sizeof(char) * 256);
27532753
/* default to port 3350 */
2754-
g_strncpy(port, "3350", port_bytes - 1);
2754+
strlcpy(port, "3350", port_bytes);
27552755
/* see if port is in sesman.ini file */
27562756
g_snprintf(cfg_file, 255, "%s/sesman.ini", XRDP_CFG_PATH);
27572757
fd = g_file_open_ro(cfg_file);
@@ -2778,7 +2778,7 @@ xrdp_mm_get_sesman_port(char *port, int port_bytes)
27782778

27792779
if ((error > 0) && (error < 65000))
27802780
{
2781-
g_strncpy(port, val, port_bytes - 1);
2781+
strlcpy(port, val, port_bytes);
27822782
}
27832783

27842784
break;
@@ -3048,7 +3048,13 @@ parse_chansrvport(const char *value, char *dest, int dest_size, int uid)
30483048
int rv = 0;
30493049
int dnum = 0;
30503050

3051-
if (g_strncmp(value, "DISPLAY(", 8) == 0)
3051+
if (value == NULL)
3052+
{
3053+
LOG(LOG_LEVEL_WARNING,
3054+
"unexpectedly empty chansrvport string encountered");
3055+
rv = -1;
3056+
}
3057+
else if (g_strncmp(value, "DISPLAY(", 8) == 0)
30523058
{
30533059
const char *p = value + 8;
30543060
const char *end = p;
@@ -3103,7 +3109,7 @@ parse_chansrvport(const char *value, char *dest, int dest_size, int uid)
31033109
}
31043110
else
31053111
{
3106-
g_strncpy(dest, value, dest_size - 1);
3112+
strlcpy(dest, value, dest_size);
31073113
}
31083114

31093115
return rv;
@@ -3354,11 +3360,16 @@ xrdp_mm_connect_sm(struct xrdp_mm *self)
33543360

33553361
gw_username = xrdp_mm_get_value(self, "pamusername");
33563362
gw_password = xrdp_mm_get_value(self, "pampassword");
3357-
if (!g_strcmp(gw_username, "same"))
3363+
// gw_username shouldn't be NULL here, but we'll
3364+
// check it anyway before dereferencing.
3365+
if (gw_username != NULL &&
3366+
!g_strcmp(gw_username, "same"))
33583367
{
33593368
gw_username = xrdp_mm_get_value(self, "username");
33603369
}
33613370

3371+
// Default the password to the usual one if the
3372+
// user hasn't specified one, or specified 'same'
33623373
if (gw_password == NULL ||
33633374
!g_strcmp(gw_password, "same"))
33643375
{
@@ -3475,7 +3486,6 @@ xrdp_mm_connect_sm(struct xrdp_mm *self)
34753486
if (self->use_chansrv)
34763487
{
34773488
char portbuff[XRDP_SOCKETS_MAXPATH];
3478-
34793489
if (self->use_sesman)
34803490
{
34813491
g_snprintf(portbuff, sizeof(portbuff),
@@ -3486,16 +3496,25 @@ xrdp_mm_connect_sm(struct xrdp_mm *self)
34863496
else
34873497
{
34883498
const char *cp = xrdp_mm_get_value(self, "chansrvport");
3489-
portbuff[0] = '\0';
3490-
parse_chansrvport(cp, portbuff, sizeof(portbuff),
3491-
self->uid);
3499+
if (parse_chansrvport(cp, portbuff, sizeof(portbuff),
3500+
self->uid) == 0)
3501+
{
3502+
xrdp_wm_log_msg(self->wm, LOG_LEVEL_INFO,
3503+
"Connecting to chansrv on %s",
3504+
portbuff);
3505+
}
3506+
else
3507+
{
3508+
// An error has already been logged
3509+
portbuff[0] = '\0';
3510+
}
3511+
}
34923512

3493-
xrdp_wm_log_msg(self->wm, LOG_LEVEL_INFO,
3494-
"Connecting to chansrv on %s",
3495-
portbuff);
3513+
if (portbuff[0] != '\0')
3514+
{
3515+
xrdp_mm_update_allowed_channels(self);
3516+
xrdp_mm_chansrv_connect(self, portbuff);
34963517
}
3497-
xrdp_mm_update_allowed_channels(self);
3498-
xrdp_mm_chansrv_connect(self, portbuff);
34993518
}
35003519
}
35013520
break;

‎xrdp/xrdp_wm.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,6 +2148,7 @@ callback(intptr_t id, int msg, intptr_t param1, intptr_t param2,
21482148
xrdp_mm_suppress_output(wm->mm, param1,
21492149
LOWORD(param2), HIWORD(param2),
21502150
LOWORD(param3), HIWORD(param3));
2151+
break;
21512152
case 0x555a:
21522153
// "yeah, up_and_running"
21532154
xrdp_mm_up_and_running(wm->mm);

0 commit comments

Comments
 (0)