Skip to content

Commit c6fb510

Browse files
committed
Coverity CID 468139
Cater for xrdp_mm_get_value() returning NULL in a couple of places. Also:- - The function parse_chansrvport() now checks that passed-in value isn't NULL. - Unnecessary uses of g_strncpy replaced with strlcpy()
1 parent ffe9ac9 commit c6fb510

File tree

1 file changed

+33
-14
lines changed

1 file changed

+33
-14
lines changed

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;

0 commit comments

Comments
 (0)