Skip to content

Commit

Permalink
Malicious RDP server security fixes
Browse files Browse the repository at this point in the history
This commit includes fixes for a set of 21 vulnerabilities in
rdesktop when a malicious RDP server is used.

All vulnerabilities was identified and reported by Eyal Itkin.

 * Add rdp_protocol_error function that is used in several fixes
 * Refactor of process_bitmap_updates
 * Fix possible integer overflow in s_check_rem() on 32bit arch
 * Fix memory corruption in process_bitmap_data - CVE-2018-8794
 * Fix remote code execution in process_bitmap_data - CVE-2018-8795
 * Fix remote code execution in process_plane - CVE-2018-8797
 * Fix Denial of Service in mcs_recv_connect_response - CVE-2018-20175
 * Fix Denial of Service in mcs_parse_domain_params - CVE-2018-20175
 * Fix Denial of Service in sec_parse_crypt_info - CVE-2018-20176
 * Fix Denial of Service in sec_recv - CVE-2018-20176
 * Fix minor information leak in rdpdr_process - CVE-2018-8791
 * Fix Denial of Service in cssp_read_tsrequest - CVE-2018-8792
 * Fix remote code execution in cssp_read_tsrequest - CVE-2018-8793
 * Fix Denial of Service in process_bitmap_data - CVE-2018-8796
 * Fix minor information leak in rdpsnd_process_ping - CVE-2018-8798
 * Fix Denial of Service in process_secondary_order - CVE-2018-8799
 * Fix remote code execution in in ui_clip_handle_data - CVE-2018-8800
 * Fix major information leak in ui_clip_handle_data - CVE-2018-20174
 * Fix memory corruption in rdp_in_unistr - CVE-2018-20177
 * Fix Denial of Service in process_demand_active - CVE-2018-20178
 * Fix remote code execution in lspci_process - CVE-2018-20179
 * Fix remote code execution in rdpsnddbg_process - CVE-2018-20180
 * Fix remote code execution in seamless_process - CVE-2018-20181
 * Fix remote code execution in seamless_process_line - CVE-2018-20182
  • Loading branch information
hean01-cendio committed Jan 16, 2019
1 parent 1f13bf5 commit 4dca546
Show file tree
Hide file tree
Showing 16 changed files with 250 additions and 71 deletions.
2 changes: 1 addition & 1 deletion asn.c
Expand Up @@ -22,7 +22,7 @@


/* Parse an ASN.1 BER header */ /* Parse an ASN.1 BER header */
RD_BOOL RD_BOOL
ber_parse_header(STREAM s, int tagval, int *length) ber_parse_header(STREAM s, int tagval, uint32 *length)
{ {
int tag, len; int tag, len;


Expand Down
8 changes: 4 additions & 4 deletions bitmap.c
Expand Up @@ -794,15 +794,15 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
replen = revcode; replen = revcode;
collen = 0; collen = 0;
} }
while (collen > 0) while (indexw < width && collen > 0)
{ {
color = CVAL(in); color = CVAL(in);
*out = color; *out = color;
out += 4; out += 4;
indexw++; indexw++;
collen--; collen--;
} }
while (replen > 0) while (indexw < width && replen > 0)
{ {
*out = color; *out = color;
out += 4; out += 4;
Expand All @@ -824,7 +824,7 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
replen = revcode; replen = revcode;
collen = 0; collen = 0;
} }
while (collen > 0) while (indexw < width && collen > 0)
{ {
x = CVAL(in); x = CVAL(in);
if (x & 1) if (x & 1)
Expand All @@ -844,7 +844,7 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
indexw++; indexw++;
collen--; collen--;
} }
while (replen > 0) while (indexw < width && replen > 0)
{ {
x = last_line[indexw * 4] + color; x = last_line[indexw * 4] + color;
*out = x; *out = x;
Expand Down
6 changes: 6 additions & 0 deletions cliprdr.c
Expand Up @@ -118,6 +118,7 @@ cliprdr_process(STREAM s)
uint16 type, status; uint16 type, status;
uint32 length, format; uint32 length, format;
uint8 *data; uint8 *data;
struct stream packet = *s;


in_uint16_le(s, type); in_uint16_le(s, type);
in_uint16_le(s, status); in_uint16_le(s, status);
Expand All @@ -127,6 +128,11 @@ cliprdr_process(STREAM s)
logger(Clipboard, Debug, "cliprdr_process(), type=%d, status=%d, length=%d", type, status, logger(Clipboard, Debug, "cliprdr_process(), type=%d, status=%d, length=%d", type, status,
length); length);


if (!s_check_rem(s, length))
{
rdp_protocol_error("cliprdr_process(), consume of packet from stream would overrun", &packet);
}

if (status == CLIPRDR_ERROR) if (status == CLIPRDR_ERROR)
{ {
switch (type) switch (type)
Expand Down
3 changes: 3 additions & 0 deletions constants.h
Expand Up @@ -751,6 +751,9 @@ enum RDP_DESKTOP_ORIENTATION
#define ENC_SALTED_CHECKSUM 0x0010 #define ENC_SALTED_CHECKSUM 0x0010
#define NO_BITMAP_COMPRESSION_HDR 0x0400 #define NO_BITMAP_COMPRESSION_HDR 0x0400


/* [MS-RDPBCGR], TS_BITMAP_DATA, flags */
#define BITMAP_COMPRESSION 0x0001

/* orderFlags, [MS-RDPBCGR] 2.2.7.1.3 */ /* orderFlags, [MS-RDPBCGR] 2.2.7.1.3 */
#define NEGOTIATEORDERSUPPORT 0x0002 #define NEGOTIATEORDERSUPPORT 0x0002
#define ZEROBOUNDSDELTASSUPPORT 0x0008 #define ZEROBOUNDSDELTASSUPPORT 0x0008
Expand Down
17 changes: 16 additions & 1 deletion cssp.c
Expand Up @@ -595,6 +595,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)
STREAM s; STREAM s;
int length; int length;
int tagval; int tagval;
struct stream packet;


s = tcp_recv(NULL, 4); s = tcp_recv(NULL, 4);


Expand Down Expand Up @@ -622,6 +623,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)


// receive the remainings of message // receive the remainings of message
s = tcp_recv(s, length); s = tcp_recv(s, length);
packet = *s;


// parse the response and into nego token // parse the response and into nego token
if (!ber_in_header(s, &tagval, &length) || if (!ber_in_header(s, &tagval, &length) ||
Expand All @@ -632,6 +634,12 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)
if (!ber_in_header(s, &tagval, &length) || if (!ber_in_header(s, &tagval, &length) ||
tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0)) tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0))
return False; return False;

if (!s_check_rem(s, length))
{
rdp_protocol_error("cssp_read_tsrequest(), consume of version from stream would overrun",
&packet);
}
in_uint8s(s, length); in_uint8s(s, length);


// negoToken [1] // negoToken [1]
Expand All @@ -653,7 +661,14 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)
if (!ber_in_header(s, &tagval, &length) || tagval != BER_TAG_OCTET_STRING) if (!ber_in_header(s, &tagval, &length) || tagval != BER_TAG_OCTET_STRING)
return False; return False;


token->end = token->p = token->data; if (!s_check_rem(s, length))
{
rdp_protocol_error("cssp_read_tsrequest(), consume of token from stream would overrun",
&packet);
}

s_realloc(token, length);
s_reset(token);
out_uint8p(token, s->p, length); out_uint8p(token, s->p, length);
s_mark_end(token); s_mark_end(token);
} }
Expand Down
9 changes: 8 additions & 1 deletion lspci.c
@@ -1,7 +1,8 @@
/* -*- c-basic-offset: 8 -*- /* -*- c-basic-offset: 8 -*-
rdesktop: A Remote Desktop Protocol client. rdesktop: A Remote Desktop Protocol client.
Support for the Matrox "lspci" channel Support for the Matrox "lspci" channel
Copyright (C) 2005 Matrox Graphics Inc. Copyright (C) 2005 Matrox Graphics Inc.
Copyright 2018 Henrik Andersson <hean01@cendio.se> for Cendio AB
This program is free software: you can redistribute it and/or modify This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -134,6 +135,12 @@ lspci_process(STREAM s)
unsigned int pkglen; unsigned int pkglen;
static char *rest = NULL; static char *rest = NULL;
char *buf; char *buf;
struct stream packet = *s;

if (!s_check(s))
{
rdp_protocol_error("lspci_process(), stream is in unstable state", &packet);
}


pkglen = s->end - s->p; pkglen = s->end - s->p;
/* str_handle_lines requires null terminated strings */ /* str_handle_lines requires null terminated strings */
Expand Down
20 changes: 18 additions & 2 deletions mcs.c
Expand Up @@ -45,9 +45,16 @@ mcs_out_domain_params(STREAM s, int max_channels, int max_users, int max_tokens,
static RD_BOOL static RD_BOOL
mcs_parse_domain_params(STREAM s) mcs_parse_domain_params(STREAM s)
{ {
int length; uint32 length;
struct stream packet = *s;


ber_parse_header(s, MCS_TAG_DOMAIN_PARAMS, &length); ber_parse_header(s, MCS_TAG_DOMAIN_PARAMS, &length);

if (!s_check_rem(s, length))
{
rdp_protocol_error("mcs_parse_domain_params(), consume domain params from stream would overrun", &packet);
}

in_uint8s(s, length); in_uint8s(s, length);


return s_check(s); return s_check(s);
Expand Down Expand Up @@ -89,8 +96,9 @@ mcs_recv_connect_response(STREAM mcs_data)
{ {
UNUSED(mcs_data); UNUSED(mcs_data);
uint8 result; uint8 result;
int length; uint32 length;
STREAM s; STREAM s;
struct stream packet;
RD_BOOL is_fastpath; RD_BOOL is_fastpath;
uint8 fastpath_hdr; uint8 fastpath_hdr;


Expand All @@ -99,6 +107,8 @@ mcs_recv_connect_response(STREAM mcs_data)


if (s == NULL) if (s == NULL)
return False; return False;

packet = *s;


ber_parse_header(s, MCS_CONNECT_RESPONSE, &length); ber_parse_header(s, MCS_CONNECT_RESPONSE, &length);


Expand All @@ -112,6 +122,12 @@ mcs_recv_connect_response(STREAM mcs_data)


ber_parse_header(s, BER_TAG_INTEGER, &length); ber_parse_header(s, BER_TAG_INTEGER, &length);
in_uint8s(s, length); /* connect id */ in_uint8s(s, length); /* connect id */

if (!s_check_rem(s, length))
{
rdp_protocol_error("mcs_recv_connect_response(), consume connect id from stream would overrun", &packet);
}

mcs_parse_domain_params(s); mcs_parse_domain_params(s);


ber_parse_header(s, BER_TAG_OCTET_STRING, &length); ber_parse_header(s, BER_TAG_OCTET_STRING, &length);
Expand Down
6 changes: 6 additions & 0 deletions orders.c
Expand Up @@ -1259,11 +1259,17 @@ process_secondary_order(STREAM s)
uint16 flags; uint16 flags;
uint8 type; uint8 type;
uint8 *next_order; uint8 *next_order;
struct stream packet = *s;


in_uint16_le(s, length); in_uint16_le(s, length);
in_uint16_le(s, flags); /* used by bmpcache2 */ in_uint16_le(s, flags); /* used by bmpcache2 */
in_uint8(s, type); in_uint8(s, type);


if (!s_check_rem(s, length + 7))
{
rdp_protocol_error("process_secondary_order(), next order pointer would overrun stream", &packet);
}

next_order = s->p + (sint16) length + 7; next_order = s->p + (sint16) length + 7;


switch (type) switch (type)
Expand Down
3 changes: 2 additions & 1 deletion proto.h
Expand Up @@ -164,6 +164,7 @@ RD_BOOL rdp_connect(char *server, uint32 flags, char *domain, char *password, ch
char *directory, RD_BOOL reconnect); char *directory, RD_BOOL reconnect);
void rdp_reset_state(void); void rdp_reset_state(void);
void rdp_disconnect(void); void rdp_disconnect(void);
void rdp_protocol_error(const char *message, STREAM s);
/* rdpdr.c */ /* rdpdr.c */
int get_device_index(RD_NTHANDLE handle); int get_device_index(RD_NTHANDLE handle);
void convert_to_unix_filename(char *filename); void convert_to_unix_filename(char *filename);
Expand Down Expand Up @@ -224,7 +225,7 @@ void tcp_run_ui(RD_BOOL run);
/* asn.c */ /* asn.c */
RD_BOOL ber_in_header(STREAM s, int *tagval, int *length); RD_BOOL ber_in_header(STREAM s, int *tagval, int *length);
void ber_out_header(STREAM s, int tagval, int length); void ber_out_header(STREAM s, int tagval, int length);
RD_BOOL ber_parse_header(STREAM s, int tagval, int *length); RD_BOOL ber_parse_header(STREAM s, int tagval, uint32 *length);
void ber_out_integer(STREAM s, int value); void ber_out_integer(STREAM s, int value);
void ber_out_sequence(STREAM s, STREAM contents); void ber_out_sequence(STREAM s, STREAM contents);


Expand Down

0 comments on commit 4dca546

Please sign in to comment.