From 836e008853c78ae5ec8932db1a6359829a1235db Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 11 Apr 2019 16:30:09 +0200 Subject: [PATCH 01/21] Fix fast path stream array There are 16 possible codes, not 15. (even if we currently don't know what to do with the last code) --- rdp5.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdp5.c b/rdp5.c index 3f41282..c5ef13b 100644 --- a/rdp5.c +++ b/rdp5.c @@ -85,7 +85,7 @@ process_ts_fp_updates(STREAM s) struct stream *ns = &(g_mppc_dict.ns); struct stream *ts; - static STREAM assembled[0x0F] = { 0 }; + static STREAM assembled[16] = { 0 }; ui_begin_update(); while (s->p < s->end) From df94870c91264ad7298dbac59f2e5f87fa70a367 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 Apr 2019 11:10:56 +0200 Subject: [PATCH 02/21] Remove unused variables and functions --- rdp.c | 1 + ssl.c | 14 -------------- tcp.c | 3 --- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/rdp.c b/rdp.c index 8cafa94..f43b3ed 100644 --- a/rdp.c +++ b/rdp.c @@ -948,6 +948,7 @@ rdp_process_virtchan_caps(STREAM s) in_uint32_le(s, flags); in_uint32_le(s, chunk_size); + UNUSED(flags); vc_chunk_size = chunk_size; } diff --git a/ssl.c b/ssl.c index 60f0b89..41cd843 100644 --- a/ssl.c +++ b/ssl.c @@ -74,20 +74,6 @@ rdssl_rc4_crypt(RDSSL_RC4 * rc4, uint8 * in_data, uint8 * out_data, uint32 len) arcfour_crypt(rc4, len, out_data, in_data); } -static void -reverse(uint8 * p, int len) -{ - int i, j; - uint8 temp; - - for (i = 0, j = len - 1; i < j; i++, j--) - { - temp = p[i]; - p[i] = p[j]; - p[j] = temp; - } -} - void rdssl_rsa_encrypt(uint8 * out, uint8 * in, int len, uint32 modulus_size, uint8 * modulus, uint8 * exponent) diff --git a/tcp.c b/tcp.c index 7f8cd10..69e74b9 100644 --- a/tcp.c +++ b/tcp.c @@ -349,9 +349,6 @@ tcp_tls_connect(void) { int err; - int type; - int status; - gnutls_datum_t out; gnutls_certificate_credentials_t xcred; /* Initialize TLS session */ From cf95138c9b87e4e61a9f1b99841402cff4d1a99b Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 Apr 2019 11:11:14 +0200 Subject: [PATCH 03/21] Don't use strncpy() when not needed It upsets the compiler warnings when you do strncpy() with the source buffer size as the limit. It is also unnecessary to use strncpy() here as we just allocated a buffer guaranteed to be large enough. --- rdesktop.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rdesktop.c b/rdesktop.c index a32362c..ce4acdc 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -836,7 +836,7 @@ main(int argc, char *argv[]) case 'u': g_username = (char *) xmalloc(strlen(optarg) + 1); - STRNCPY(g_username, optarg, strlen(optarg) + 1); + strcpy(g_username, optarg); username_option = 1; break; @@ -1355,7 +1355,7 @@ main(int argc, char *argv[]) STRNCPY(domain, g_redirect_domain, sizeof(domain)); xfree(g_username); g_username = (char *) xmalloc(strlen(g_redirect_username) + 1); - STRNCPY(g_username, g_redirect_username, strlen(g_redirect_username) + 1); + strcpy(g_username, g_redirect_username); STRNCPY(server, g_redirect_server, sizeof(server)); flags |= RDP_INFO_AUTOLOGON; From 48b184477ee12cfc2cdeb3975231c011e415e22c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 Apr 2019 11:13:48 +0200 Subject: [PATCH 04/21] Fix bad call to strncat() --- rdesktop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdesktop.c b/rdesktop.c index ce4acdc..4148140 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -1790,7 +1790,7 @@ str_handle_lines(const char *input, char **rest, str_handle_lines_t linehandler, buf[0] = '\0'; if (*rest) STRNCPY(buf, *rest, buflen); - strncat(buf, input, inputlen); + strncat(buf, input, buflen); p = buf; while (1) From 092fc209219ee6ca6ce3d98e163bfc77d8624532 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 Apr 2019 11:14:12 +0200 Subject: [PATCH 05/21] Use STRNCPY() macro in smart card code The previous code did not do a proper bounds check and could result in buffer overflows and unterminated strings if long names were specified. --- scard.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scard.c b/scard.c index 468bdd4..6178d6e 100644 --- a/scard.c +++ b/scard.c @@ -237,10 +237,8 @@ scard_enum_devices(uint32 * id, char *optarg) tmpMap = nameMapList + nameMapCount - 1; - len = strlen(alias); - strncpy(tmpMap->alias, alias, (len > 127) ? (127) : (len)); - len = strlen(name); - strncpy(tmpMap->name, name, (len > 127) ? (127) : (len)); + STRNCPY(tmpMap->alias, alias, sizeof(tmpMap->alias)); + STRNCPY(tmpMap->name, name, sizeof(tmpMap->name)); if (vendor) { @@ -248,8 +246,8 @@ scard_enum_devices(uint32 * id, char *optarg) if (len > 0) { memset(tmpMap->vendor, 0, 128); - strncpy(tmpMap->vendor, vendor, - (len > 127) ? (127) : (len)); + STRNCPY(tmpMap->vendor, vendor, + sizeof(tmpMap->vendor)); } else tmpMap->vendor[0] = '\0'; From d8b0f3782a12f92fe6ca606f05b3753f4e79f342 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 14:02:42 +0200 Subject: [PATCH 06/21] Give source code info in rdp_protocol_error() Makes it easier to debug things by seeing exactly where the protocol handling crashed. --- cliprdr.c | 2 +- cssp.c | 4 ++-- mcs.c | 4 ++-- orders.c | 2 +- proto.h | 4 +++- rdp.c | 11 ++++++----- rdpsnd.c | 2 +- secure.c | 6 +++--- 8 files changed, 19 insertions(+), 16 deletions(-) diff --git a/cliprdr.c b/cliprdr.c index 76eb98d..337b91b 100644 --- a/cliprdr.c +++ b/cliprdr.c @@ -130,7 +130,7 @@ cliprdr_process(STREAM s) if (!s_check_rem(s, length)) { - rdp_protocol_error("cliprdr_process(), consume of packet from stream would overrun", &packet); + rdp_protocol_error("consume of packet from stream would overrun", &packet); } if (status == CLIPRDR_ERROR) diff --git a/cssp.c b/cssp.c index 6b2d0f7..7e4f611 100644 --- a/cssp.c +++ b/cssp.c @@ -637,7 +637,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) if (!s_check_rem(s, length)) { - rdp_protocol_error("cssp_read_tsrequest(), consume of version from stream would overrun", + rdp_protocol_error("consume of version from stream would overrun", &packet); } in_uint8s(s, length); @@ -663,7 +663,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) if (!s_check_rem(s, length)) { - rdp_protocol_error("cssp_read_tsrequest(), consume of token from stream would overrun", + rdp_protocol_error("consume of token from stream would overrun", &packet); } diff --git a/mcs.c b/mcs.c index 0f2360e..aeeb675 100644 --- a/mcs.c +++ b/mcs.c @@ -52,7 +52,7 @@ mcs_parse_domain_params(STREAM s) if (!s_check_rem(s, length)) { - rdp_protocol_error("mcs_parse_domain_params(), consume domain params from stream would overrun", &packet); + rdp_protocol_error("consume domain params from stream would overrun", &packet); } in_uint8s(s, length); @@ -125,7 +125,7 @@ mcs_recv_connect_response(STREAM mcs_data) if (!s_check_rem(s, length)) { - rdp_protocol_error("mcs_recv_connect_response(), consume connect id from stream would overrun", &packet); + rdp_protocol_error("consume connect id from stream would overrun", &packet); } mcs_parse_domain_params(s); diff --git a/orders.c b/orders.c index 40d4356..bcc1a7e 100644 --- a/orders.c +++ b/orders.c @@ -1272,7 +1272,7 @@ process_secondary_order(STREAM s) if (!s_check_rem(s, length)) { - rdp_protocol_error("process_secondary_order(), next order pointer would overrun stream", &packet); + rdp_protocol_error("next order pointer would overrun stream", &packet); } next_order = s->p + length; diff --git a/proto.h b/proto.h index bfb046a..5571996 100644 --- a/proto.h +++ b/proto.h @@ -165,7 +165,9 @@ RD_BOOL rdp_connect(char *server, uint32 flags, char *domain, char *password, ch char *directory, RD_BOOL reconnect); void rdp_reset_state(void); void rdp_disconnect(void); -void rdp_protocol_error(const char *message, STREAM s); +#define rdp_protocol_error(m, s) _rdp_protocol_error(__FILE__, __LINE__, __func__, m, s) +void _rdp_protocol_error(const char *file, int line, const char *func, + const char *message, STREAM s); /* rdpdr.c */ int get_device_index(RD_NTHANDLE handle); void convert_to_unix_filename(char *filename); diff --git a/rdp.c b/rdp.c index f43b3ed..ec56c9a 100644 --- a/rdp.c +++ b/rdp.c @@ -298,7 +298,7 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) if (!s_check_rem(s, in_len)) { - rdp_protocol_error("rdp_in_unistr(), consume of unicode data from stream would overrun", &packet); + rdp_protocol_error("consume of unicode data from stream would overrun", &packet); } // if not already open @@ -1269,7 +1269,7 @@ process_demand_active(STREAM s) if (!s_check_rem(s, len_src_descriptor)) { - rdp_protocol_error("rdp_demand_active(), consume of source descriptor from stream would overrun", &packet); + rdp_protocol_error("consume of source descriptor from stream would overrun", &packet); } in_uint8s(s, len_src_descriptor); @@ -1522,7 +1522,7 @@ process_bitmap_data(STREAM s) /* read compressed bitmap data */ if (!s_check_rem(s, size)) { - rdp_protocol_error("process_bitmap_data(), consume of bitmap data from stream would overrun", &packet); + rdp_protocol_error("consume of bitmap data from stream would overrun", &packet); } in_uint8p(s, data, size); bmpdata = (uint8 *) xmalloc(width * height * Bpp); @@ -2112,9 +2112,10 @@ rdp_disconnect(void) */ void -rdp_protocol_error(const char *message, STREAM s) +_rdp_protocol_error(const char *file, int line, const char *func, + const char *message, STREAM s) { - logger(Protocol, Error, "%s(), %s", __func__, message); + logger(Protocol, Error, "%s:%d: %s(), %s", file, line, func, message); if (s) hexdump(s->p, s_length(s)); exit(0); diff --git a/rdpsnd.c b/rdpsnd.c index 40881b7..73428c9 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -273,7 +273,7 @@ rdpsnd_process_training(STREAM in) if (!s_check_rem(in, 4)) { - rdp_protocol_error("rdpsnd_process_training(), consume of training data from stream would overrun", &packet); + rdp_protocol_error("consume of training data from stream would overrun", &packet); } in_uint16_le(in, tick); diff --git a/secure.c b/secure.c index 175e44d..5dd686b 100644 --- a/secure.c +++ b/secure.c @@ -873,7 +873,7 @@ sec_recv(RD_BOOL * is_fastpath) if (fastpath_flags & FASTPATH_OUTPUT_ENCRYPTED) { if (!s_check_rem(s, 8)) { - rdp_protocol_error("sec_recv(), consume fastpath signature from stream would overrun", &packet); + rdp_protocol_error("consume fastpath signature from stream would overrun", &packet); } in_uint8s(s, 8); /* signature */ @@ -893,7 +893,7 @@ sec_recv(RD_BOOL * is_fastpath) if (sec_flags & SEC_ENCRYPT) { if (!s_check_rem(s, 8)) { - rdp_protocol_error("sec_recv(), consume encrypt signature from stream would overrun", &packet); + rdp_protocol_error("consume encrypt signature from stream would overrun", &packet); } in_uint8s(s, 8); /* signature */ @@ -911,7 +911,7 @@ sec_recv(RD_BOOL * is_fastpath) uint8 swapbyte; if (!s_check_rem(s, 8)) { - rdp_protocol_error("sec_recv(), consume redirect signature from stream would overrun", &packet); + rdp_protocol_error("consume redirect signature from stream would overrun", &packet); } in_uint8s(s, 8); /* signature */ From e1537061bf26ecfaee221f52c69db49153650d42 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 14:03:16 +0200 Subject: [PATCH 07/21] Fix packet debug output in rdp_protocol_error() We're trying to print the entire packet, not just what's left. --- rdp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rdp.c b/rdp.c index ec56c9a..0c9be79 100644 --- a/rdp.c +++ b/rdp.c @@ -2117,6 +2117,6 @@ _rdp_protocol_error(const char *file, int line, const char *func, { logger(Protocol, Error, "%s:%d: %s(), %s", file, line, func, message); if (s) - hexdump(s->p, s_length(s)); + hexdump(s->data, s_length(s)); exit(0); } From de59a100ead9eadc5a2a925124cfb82707ed0360 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 4 Apr 2019 15:00:35 +0200 Subject: [PATCH 08/21] Mark rd_protocol_error() as "noreturn" This allows the compiler to optimize things better and give better warnings as it knows it will never return from this function. --- proto.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/proto.h b/proto.h index 5571996..f7f0c6c 100644 --- a/proto.h +++ b/proto.h @@ -25,6 +25,11 @@ extern "C" { #endif /* *INDENT-ON* */ #define UNUSED(param) ((void)param) +#ifdef __GNUC__ +# define NORETURN __attribute__((noreturn)) +#else +# define NORETURN +#endif // __GNUC__ /* bitmap.c */ RD_BOOL bitmap_decompress(uint8 * output, int width, int height, uint8 * input, int size, int Bpp); /* cache.c */ @@ -167,7 +172,7 @@ void rdp_reset_state(void); void rdp_disconnect(void); #define rdp_protocol_error(m, s) _rdp_protocol_error(__FILE__, __LINE__, __func__, m, s) void _rdp_protocol_error(const char *file, int line, const char *func, - const char *message, STREAM s); + const char *message, STREAM s) NORETURN; /* rdpdr.c */ int get_device_index(RD_NTHANDLE handle); void convert_to_unix_filename(char *filename); From e099d79879fa3662384d661a25593c661db4f823 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 4 Apr 2019 14:58:15 +0200 Subject: [PATCH 09/21] Remove dangerous s_check() This macro checks if a pointer is valid _after_ we've already used that pointer. So it will only trigger if we're already performed some for of buffer overflow. As such, it provides little to no value and can only server to encourage broken behaviour. Let's remove it and replace it with proper bounds checking before access instead. --- asn.c | 14 +++++++++++++- lspci.c | 6 ------ mcs.c | 4 +--- orders.c | 12 +++--------- rdp.c | 3 +-- rdp5.c | 3 +-- rdpdr.c | 10 ---------- rdpsnd.c | 5 ----- seamless.c | 6 ------ secure.c | 10 +++++++++- stream.h | 3 +-- 11 files changed, 29 insertions(+), 47 deletions(-) diff --git a/asn.c b/asn.c index bd0577b..6c591c8 100644 --- a/asn.c +++ b/asn.c @@ -41,10 +41,16 @@ ber_parse_header(STREAM s, int tagval, uint32 *length) if (tagval > 0xff) { + if (!s_check_rem(s, 2)) { + return False; + } in_uint16_be(s, tag); } else { + if (!s_check_rem(s, 1)) { + return False; + } in_uint8(s, tag); } @@ -54,11 +60,17 @@ ber_parse_header(STREAM s, int tagval, uint32 *length) return False; } + if (!s_check_rem(s, 1)) { + return False; + } in_uint8(s, len); if (len & 0x80) { len &= ~0x80; + if (!s_check_rem(s, len)) { + return False; + } *length = 0; while (len--) next_be(s, *length); @@ -66,7 +78,7 @@ ber_parse_header(STREAM s, int tagval, uint32 *length) else *length = len; - return s_check(s); + return True; } void diff --git a/lspci.c b/lspci.c index 2d17161..7690a1f 100644 --- a/lspci.c +++ b/lspci.c @@ -135,12 +135,6 @@ lspci_process(STREAM s) unsigned int pkglen; static char *rest = NULL; 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; /* str_handle_lines requires null terminated strings */ diff --git a/mcs.c b/mcs.c index aeeb675..ed466a0 100644 --- a/mcs.c +++ b/mcs.c @@ -42,7 +42,7 @@ mcs_out_domain_params(STREAM s, int max_channels, int max_users, int max_tokens, } /* Parse a DOMAIN_PARAMS structure (ASN.1 BER) */ -static RD_BOOL +static void mcs_parse_domain_params(STREAM s) { uint32 length; @@ -56,8 +56,6 @@ mcs_parse_domain_params(STREAM s) } in_uint8s(s, length); - - return s_check(s); } /* Send an MCS_CONNECT_INITIAL message (ASN.1 BER) */ diff --git a/orders.c b/orders.c index bcc1a7e..44e5034 100644 --- a/orders.c +++ b/orders.c @@ -101,7 +101,7 @@ rdp_in_colour(STREAM s, uint32 * colour) } /* Parse bounds information */ -static RD_BOOL +static void rdp_parse_bounds(STREAM s, BOUNDS * bounds) { uint8 present; @@ -127,12 +127,10 @@ rdp_parse_bounds(STREAM s, BOUNDS * bounds) rdp_in_coord(s, &bounds->bottom, False); else if (present & 128) rdp_in_coord(s, &bounds->bottom, True); - - return s_check(s); } /* Parse a pen */ -static RD_BOOL +static void rdp_parse_pen(STREAM s, PEN * pen, uint32 present) { if (present & 1) @@ -143,8 +141,6 @@ rdp_parse_pen(STREAM s, PEN * pen, uint32 present) if (present & 4) rdp_in_colour(s, &pen->colour); - - return s_check(s); } static void @@ -176,7 +172,7 @@ setup_brush(BRUSH * out_brush, BRUSH * in_brush) } /* Parse a brush */ -static RD_BOOL +static void rdp_parse_brush(STREAM s, BRUSH * brush, uint32 present) { if (present & 1) @@ -193,8 +189,6 @@ rdp_parse_brush(STREAM s, BRUSH * brush, uint32 present) if (present & 16) in_uint8a(s, &brush->pattern[1], 7); - - return s_check(s); } /* Process a destination blt order */ diff --git a/rdp.c b/rdp.c index 0c9be79..2b12a01 100644 --- a/rdp.c +++ b/rdp.c @@ -1417,8 +1417,7 @@ process_pointer_pdu(STREAM s) case RDP_POINTER_MOVE: in_uint16_le(s, x); in_uint16_le(s, y); - if (s_check(s)) - ui_move_pointer(x, y); + ui_move_pointer(x, y); break; case RDP_POINTER_COLOR: diff --git a/rdp5.c b/rdp5.c index c5ef13b..5fce5f3 100644 --- a/rdp5.c +++ b/rdp5.c @@ -56,8 +56,7 @@ process_ts_fp_update_by_code(STREAM s, uint8 code) case FASTPATH_UPDATETYPE_PTR_POSITION: in_uint16_le(s, x); in_uint16_le(s, y); - if (s_check(s)) - ui_move_pointer(x, y); + ui_move_pointer(x, y); break; case FASTPATH_UPDATETYPE_COLOR: process_colour_pointer_pdu(s); diff --git a/rdpdr.c b/rdpdr.c index 402d94b..ac6ecff 100644 --- a/rdpdr.c +++ b/rdpdr.c @@ -877,7 +877,6 @@ rdpdr_process(STREAM s) uint16 vmin; uint16 component; uint16 pakid; - struct stream packet = *s; logger(Protocol, Debug, "rdpdr_process()"); /* hexdump(s->p, s->end - s->p); */ @@ -900,15 +899,6 @@ rdpdr_process(STREAM s) in_uint32_le(s, g_client_id); /* ClientID */ - /* g_client_id is sent back to server, - so lets check that we actually got - valid data from stream to prevent - that we leak back data to server */ - if (!s_check(s)) - { - rdp_protocol_error("rdpdr_process(), consume of g_client_id from stream did overrun", &packet); - } - /* The RDP client is responsibility to provide a random client id if server version is < 12 */ if (vmin < 0x000c) diff --git a/rdpsnd.c b/rdpsnd.c index 73428c9..9289380 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -451,11 +451,6 @@ rdpsnddbg_process(STREAM s) static char *rest = NULL; char *buf; - if (!s_check(s)) - { - rdp_protocol_error("rdpsnddbg_process(), stream is in unstable state", s); - } - pkglen = s->end - s->p; /* str_handle_lines requires null terminated strings */ buf = (char *) xmalloc(pkglen + 1); diff --git a/seamless.c b/seamless.c index 1935074..0f294cc 100644 --- a/seamless.c +++ b/seamless.c @@ -376,12 +376,6 @@ seamless_process(STREAM s) { unsigned int pkglen; char *buf; - struct stream packet = *s; - - if (!s_check(s)) - { - rdp_protocol_error("seamless_process(), stream is in unstable state", &packet); - } pkglen = s->end - s->p; /* str_handle_lines requires null terminated strings */ diff --git a/secure.c b/secure.c index 5dd686b..16755ff 100644 --- a/secure.c +++ b/secure.c @@ -523,6 +523,10 @@ sec_parse_public_key(STREAM s, uint8 * modulus, uint8 * exponent) { uint32 magic, modulus_len; + if (!s_check_rem(s, 8)) { + return False; + } + in_uint32_le(s, magic); if (magic != SEC_RSA_MAGIC) { @@ -541,13 +545,17 @@ sec_parse_public_key(STREAM s, uint8 * modulus, uint8 * exponent) return False; } + if (!s_check_rem(s, 1 + SEC_EXPONENT_SIZE + modulus_len + SEC_PADDING_SIZE)) { + return False; + } + in_uint8s(s, 8); /* modulus_bits, unknown */ in_uint8a(s, exponent, SEC_EXPONENT_SIZE); in_uint8a(s, modulus, modulus_len); in_uint8s(s, SEC_PADDING_SIZE); g_server_public_key_len = modulus_len; - return s_check(s); + return True; } /* Parse a public signature structure */ diff --git a/stream.h b/stream.h index f47dab7..5b2179f 100644 --- a/stream.h +++ b/stream.h @@ -55,8 +55,7 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define s_push_layer(s,h,n) { (s)->h = (s)->p; (s)->p += n; } #define s_pop_layer(s,h) (s)->p = (s)->h; #define s_mark_end(s) (s)->end = (s)->p; -#define s_check(s) ((s)->p <= (s)->end) -#define s_check_rem(s,n) (s_check(s) && (n <= (s)->end - (s)->p)) +#define s_check_rem(s,n) (((s)->p <= (s)->end) && (n <= (s)->end - (s)->p)) #define s_check_end(s) ((s)->p == (s)->end) #define s_length(s) ((s)->end - (s)->data) #define s_left(s) ((s)->size - ((s)->p - (s)->data)) From 90219aac4118a55536d940241f741687dacdeac8 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 4 Apr 2019 15:44:27 +0200 Subject: [PATCH 10/21] Add bounds checks to stream handling Protect against buffer overflow and overrun bugs in the protocol handling. --- stream.h | 72 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/stream.h b/stream.h index 5b2179f..c0cd6ee 100644 --- a/stream.h +++ b/stream.h @@ -55,38 +55,42 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define s_push_layer(s,h,n) { (s)->h = (s)->p; (s)->p += n; } #define s_pop_layer(s,h) (s)->p = (s)->h; #define s_mark_end(s) (s)->end = (s)->p; -#define s_check_rem(s,n) (((s)->p <= (s)->end) && (n <= (s)->end - (s)->p)) +#define s_check_rem(s,n) (((s)->p <= (s)->end) && ((size_t)n <= (size_t)((s)->end - (s)->p))) #define s_check_end(s) ((s)->p == (s)->end) #define s_length(s) ((s)->end - (s)->data) -#define s_left(s) ((s)->size - ((s)->p - (s)->data)) +#define s_left(s) ((s)->size - (size_t)((s)->p - (s)->data)) + +/* Verify that there is enough data/space before accessing a STREAM */ +#define s_assert_r(s,n) { if (!s_check_rem(s, n)) rdp_protocol_error( "unexpected stream overrun", s); } +#define s_assert_w(s,n) { if (s_left(s) < (size_t)n) { logger(Core, Error, "%s:%d: %s(), %s", __FILE__, __LINE__, __func__, "unexpected stream overrun"); exit(0); } } #if defined(L_ENDIAN) && !defined(NEED_ALIGN) -#define in_uint16_le(s,v) { v = *(uint16 *)((s)->p); (s)->p += 2; } -#define in_uint32_le(s,v) { v = *(uint32 *)((s)->p); (s)->p += 4; } -#define in_uint64_le(s,v) { v = *(uint64 *)((s)->p); (s)->p += 8; } -#define out_uint16_le(s,v) { *(uint16 *)((s)->p) = v; (s)->p += 2; } -#define out_uint32_le(s,v) { *(uint32 *)((s)->p) = v; (s)->p += 4; } -#define out_uint64_le(s,v) { *(uint64 *)((s)->p) = v; (s)->p += 8; } +#define in_uint16_le(s,v) { s_assert_r(s, 2); v = *(uint16 *)((s)->p); (s)->p += 2; } +#define in_uint32_le(s,v) { s_assert_r(s, 4); v = *(uint32 *)((s)->p); (s)->p += 4; } +#define in_uint64_le(s,v) { s_assert_r(s, 8); v = *(uint64 *)((s)->p); (s)->p += 8; } +#define out_uint16_le(s,v) { s_assert_w(s, 2); *(uint16 *)((s)->p) = v; (s)->p += 2; } +#define out_uint32_le(s,v) { s_assert_w(s, 4); *(uint32 *)((s)->p) = v; (s)->p += 4; } +#define out_uint64_le(s,v) { s_assert_w(s, 8); *(uint64 *)((s)->p) = v; (s)->p += 8; } #else -#define in_uint16_le(s,v) { v = *((s)->p++); v += *((s)->p++) << 8; } -#define in_uint32_le(s,v) { in_uint16_le(s,v) \ +#define in_uint16_le(s,v) { s_assert_r(s, 2); v = *((s)->p++); v += *((s)->p++) << 8; } +#define in_uint32_le(s,v) { s_assert_r(s, 4); in_uint16_le(s,v) \ v += *((s)->p++) << 16; v += *((s)->p++) << 24; } -#define in_uint64_le(s,v) { in_uint32_le(s,v) \ +#define in_uint64_le(s,v) { s_assert_r(s, 8); in_uint32_le(s,v) \ v += *((s)->p++) << 32; v += *((s)->p++) << 40; \ v += *((s)->p++) << 48; v += *((s)->p++) << 56; } -#define out_uint16_le(s,v) { *((s)->p++) = (v) & 0xff; *((s)->p++) = ((v) >> 8) & 0xff; } -#define out_uint32_le(s,v) { out_uint16_le(s, (v) & 0xffff); out_uint16_le(s, ((v) >> 16) & 0xffff); } -#define out_uint64_le(s,v) { out_uint32_le(s, (v) & 0xffffffff); out_uint32_le(s, ((v) >> 32) & 0xffffffff); } +#define out_uint16_le(s,v) { s_assert_w(s, 2); *((s)->p++) = (v) & 0xff; *((s)->p++) = ((v) >> 8) & 0xff; } +#define out_uint32_le(s,v) { s_assert_w(s, 4); out_uint16_le(s, (v) & 0xffff); out_uint16_le(s, ((v) >> 16) & 0xffff); } +#define out_uint64_le(s,v) { s_assert_w(s, 8); out_uint32_le(s, (v) & 0xffffffff); out_uint32_le(s, ((v) >> 32) & 0xffffffff); } #endif #if defined(B_ENDIAN) && !defined(NEED_ALIGN) -#define in_uint16_be(s,v) { v = *(uint16 *)((s)->p); (s)->p += 2; } -#define in_uint32_be(s,v) { v = *(uint32 *)((s)->p); (s)->p += 4; } -#define in_uint64_be(s,v) { v = *(uint64 *)((s)->p); (s)->p += 8; } -#define out_uint16_be(s,v) { *(uint16 *)((s)->p) = v; (s)->p += 2; } -#define out_uint32_be(s,v) { *(uint32 *)((s)->p) = v; (s)->p += 4; } -#define out_uint64_be(s,v) { *(uint64 *)((s)->p) = v; (s)->p += 8; } +#define in_uint16_be(s,v) { s_assert_r(s, 2); v = *(uint16 *)((s)->p); (s)->p += 2; } +#define in_uint32_be(s,v) { s_assert_r(s, 4); v = *(uint32 *)((s)->p); (s)->p += 4; } +#define in_uint64_be(s,v) { s_assert_r(s, 8); v = *(uint64 *)((s)->p); (s)->p += 8; } +#define out_uint16_be(s,v) { s_assert_w(s, 2); *(uint16 *)((s)->p) = v; (s)->p += 2; } +#define out_uint32_be(s,v) { s_assert_w(s, 4); *(uint32 *)((s)->p) = v; (s)->p += 4; } +#define out_uint64_be(s,v) { s_assert_w(s, 8); *(uint64 *)((s)->p) = v; (s)->p += 8; } #define B_ENDIAN_PREFERRED #define in_uint16(s,v) in_uint16_be(s,v) @@ -98,12 +102,12 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define out_uint64(s,v) out_uint64_be(s,v) #else -#define in_uint16_be(s,v) { v = *((s)->p++); next_be(s,v); } -#define in_uint32_be(s,v) { in_uint16_be(s,v); next_be(s,v); next_be(s,v); } -#define in_uint64_be(s,v) { in_uint32_be(s,v); next_be(s,v); next_be(s,v); next_be(s,v); next_be(s,v); } -#define out_uint16_be(s,v) { *((s)->p++) = ((v) >> 8) & 0xff; *((s)->p++) = (v) & 0xff; } -#define out_uint32_be(s,v) { out_uint16_be(s, ((v) >> 16) & 0xffff); out_uint16_be(s, (v) & 0xffff); } -#define out_uint64_be(s,v) { out_uint32_be(s, ((v) >> 32) & 0xffffffff); out_uint32_be(s, (v) & 0xffffffff); } +#define in_uint16_be(s,v) { s_assert_r(s, 2); v = *((s)->p++); next_be(s,v); } +#define in_uint32_be(s,v) { s_assert_r(s, 4); in_uint16_be(s,v); next_be(s,v); next_be(s,v); } +#define in_uint64_be(s,v) { s_assert_r(s, 8); in_uint32_be(s,v); next_be(s,v); next_be(s,v); next_be(s,v); next_be(s,v); } +#define out_uint16_be(s,v) { s_assert_w(s, 2); *((s)->p++) = ((v) >> 8) & 0xff; *((s)->p++) = (v) & 0xff; } +#define out_uint32_be(s,v) { s_assert_w(s, 4); out_uint16_be(s, ((v) >> 16) & 0xffff); out_uint16_be(s, (v) & 0xffff); } +#define out_uint64_be(s,v) { s_assert_w(s, 8); out_uint32_be(s, ((v) >> 32) & 0xffffffff); out_uint32_be(s, (v) & 0xffffffff); } #endif #ifndef B_ENDIAN_PREFERRED @@ -115,18 +119,18 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define out_uint64(s,v) out_uint64_le(s,v) #endif -#define in_uint8(s,v) v = *((s)->p++); -#define in_uint8p(s,v,n) { v = (s)->p; (s)->p += n; } -#define in_uint8a(s,v,n) { memcpy(v,(s)->p,n); (s)->p += n; } -#define in_uint8s(s,n) (s)->p += n; +#define in_uint8(s,v) { s_assert_r(s, 1); v = *((s)->p++); } +#define in_uint8p(s,v,n) { s_assert_r(s, n); v = (s)->p; (s)->p += n; } +#define in_uint8a(s,v,n) { s_assert_r(s, n); memcpy(v,(s)->p,n); (s)->p += n; } +#define in_uint8s(s,n) { s_assert_r(s, n); (s)->p += n; } #define in_skip(s,n) in_uint8s(s,n) -#define out_uint8(s,v) *((s)->p++) = v; -#define out_uint8p(s,v,n) { memcpy((s)->p,v,n); (s)->p += n; } +#define out_uint8(s,v) { s_assert_w(s, 1); *((s)->p++) = v; } +#define out_uint8p(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } #define out_uint8a(s,v,n) out_uint8p(s,v,n); -#define out_uint8s(s,n) { memset((s)->p,0,n); (s)->p += n; } +#define out_uint8s(s,n) { s_assert_w(s, n); memset((s)->p,0,n); (s)->p += n; } #define out_stream(s, v) out_uint8p(s, (v)->data, s_length((v))) -#define next_be(s,v) v = ((v) << 8) + *((s)->p++); +#define next_be(s,v) { s_assert_r(s, 1); v = ((v) << 8) + *((s)->p++); } #endif /* _STREAM_H */ From dd0217f372650e0d04c1e6b5a8f74e975fb30ef7 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 4 Apr 2019 15:45:52 +0200 Subject: [PATCH 11/21] Remove in_skip() in favour of in_uint8s() It was barely used and it was confusing having two macros doing the same thing. Standardise on the more common variant. --- rdpedisp.c | 2 +- stream.h | 1 - tests/resize_test.c | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rdpedisp.c b/rdpedisp.c index fa5c7a2..746034b 100644 --- a/rdpedisp.c +++ b/rdpedisp.c @@ -64,7 +64,7 @@ rdpedisp_process_pdu(STREAM s) /* Read DISPLAYCONTROL_HEADER */ in_uint32_le(s, type); /* type */ - in_skip(s, 4); /* length */ + in_uint8s(s, 4); /* length */ logger(Protocol, Debug, "rdpedisp_process_pdu(), Got PDU type %d", type); diff --git a/stream.h b/stream.h index c0cd6ee..228b648 100644 --- a/stream.h +++ b/stream.h @@ -123,7 +123,6 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define in_uint8p(s,v,n) { s_assert_r(s, n); v = (s)->p; (s)->p += n; } #define in_uint8a(s,v,n) { s_assert_r(s, n); memcpy(v,(s)->p,n); (s)->p += n; } #define in_uint8s(s,n) { s_assert_r(s, n); (s)->p += n; } -#define in_skip(s,n) in_uint8s(s,n) #define out_uint8(s,v) { s_assert_w(s, 1); *((s)->p++) = v; } #define out_uint8p(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } #define out_uint8a(s,v,n) out_uint8p(s,v,n); diff --git a/tests/resize_test.c b/tests/resize_test.c index 80da9b8..973d1fc 100644 --- a/tests/resize_test.c +++ b/tests/resize_test.c @@ -433,7 +433,7 @@ void get_width_and_height_from_mcs_connect_initial(int *width, int *height) /* Rewind and extract the requested session size */ s_reset(s); - in_skip(s, 31); + in_uint8s(s, 31); in_uint16_le(s, *width); /* desktopWidth */ in_uint16_le(s, *height); /* desktopHeight */ From 3e340f2f200861dafea480e6bf2d96b1e44f0202 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 12:20:27 +0200 Subject: [PATCH 12/21] Add explicit STREAM allocation function Avoids mistakes by making sure everyone allocates these the same way. The smart card code still has manual allocation because it has it's own magical memory management. --- cssp.c | 7 +------ mcs.c | 5 +---- rdp5.c | 6 +----- scard.c | 1 + secure.c | 11 +++++------ stream.c | 12 ++++++++++++ stream.h | 5 +++++ tests/asn_test.c | 24 +++++++----------------- tests/mcs_test.c | 13 ++----------- tests/resize_test.c | 11 ----------- 10 files changed, 35 insertions(+), 60 deletions(-) diff --git a/cssp.c b/cssp.c index 7e4f611..98237d8 100644 --- a/cssp.c +++ b/cssp.c @@ -36,12 +36,7 @@ ber_wrap_hdr_data(int tagval, STREAM in) STREAM out; int size = s_length(in) + 16; - out = xmalloc(sizeof(struct stream)); - memset(out, 0, sizeof(struct stream)); - out->data = xmalloc(size); - out->size = size; - out->p = out->data; - + out = s_alloc(size); ber_out_header(out, tagval, s_length(in)); out_uint8p(out, in->data, s_length(in)); s_mark_end(out); diff --git a/mcs.c b/mcs.c index ed466a0..67ec178 100644 --- a/mcs.c +++ b/mcs.c @@ -273,10 +273,7 @@ mcs_send_dpu(unsigned short reason) logger(Protocol, Debug, "mcs_send_dpu(), reason=%d", reason); - contents = malloc(sizeof(struct stream)); - memset(contents, 0, sizeof(struct stream)); - s_realloc(contents, 6); - s_reset(contents); + contents = s_alloc(6); ber_out_integer(contents, reason); /* Reason */ ber_out_sequence(contents, NULL); /* SEQUENCE OF NonStandradParameters OPTIONAL */ s_mark_end(contents); diff --git a/rdp5.c b/rdp5.c index 5fce5f3..9e8790b 100644 --- a/rdp5.c +++ b/rdp5.c @@ -132,11 +132,7 @@ process_ts_fp_updates(STREAM s) { if (assembled[code] == NULL) { - assembled[code] = xmalloc(sizeof(struct stream)); - memset(assembled[code], 0, sizeof(struct stream)); - s_realloc(assembled[code], - RDESKTOP_FASTPATH_MULTIFRAGMENT_MAX_SIZE); - s_reset(assembled[code]); + assembled[code] = s_alloc(RDESKTOP_FASTPATH_MULTIFRAGMENT_MAX_SIZE); } if (frag == FASTPATH_FRAGMENT_FIRST) diff --git a/scard.c b/scard.c index 6178d6e..52d94b9 100644 --- a/scard.c +++ b/scard.c @@ -2438,6 +2438,7 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out, static STREAM duplicateStream(PMEM_HANDLE * handle, STREAM s, uint32 buffer_size, RD_BOOL isInputStream) { + // FIXME: Shouldn't be allocating streams manually STREAM d = SC_xmalloc(handle, sizeof(struct stream)); if (d != NULL) { diff --git a/secure.c b/secure.c index 16755ff..980d5b2 100644 --- a/secure.c +++ b/secure.c @@ -976,25 +976,24 @@ RD_BOOL sec_connect(char *server, char *username, char *domain, char *password, RD_BOOL reconnect) { uint32 selected_proto; - struct stream mcs_data; + STREAM mcs_data; /* Start a MCS connect sequence */ if (!mcs_connect_start(server, username, domain, password, reconnect, &selected_proto)) return False; /* We exchange some RDP data during the MCS-Connect */ - mcs_data.size = 512; - mcs_data.p = mcs_data.data = (uint8 *) xmalloc(mcs_data.size); - sec_out_mcs_connect_initial_pdu(&mcs_data, selected_proto); + mcs_data = s_alloc(512); + sec_out_mcs_connect_initial_pdu(mcs_data, selected_proto); /* finalize the MCS connect sequence */ - if (!mcs_connect_finalize(&mcs_data)) + if (!mcs_connect_finalize(mcs_data)) return False; /* sec_process_mcs_data(&mcs_data); */ if (g_encryption) sec_establish_key(); - xfree(mcs_data.data); + s_free(mcs_data); return True; } diff --git a/stream.c b/stream.c index f66158f..8400446 100644 --- a/stream.c +++ b/stream.c @@ -25,6 +25,18 @@ extern char g_codepage[16]; +STREAM +s_alloc(unsigned int size) +{ + STREAM s; + + s = xmalloc(sizeof(struct stream)); + memset(s, 0, sizeof(struct stream)); + s_realloc(s, size); + + return s; +} + void s_realloc(STREAM s, unsigned int size) { diff --git a/stream.h b/stream.h index 228b648..ec61f0e 100644 --- a/stream.h +++ b/stream.h @@ -41,8 +41,13 @@ typedef struct stream } *STREAM; +/* Return a newly allocated STREAM object of the specified size */ +STREAM s_alloc(unsigned int size); +/* Resize an existing STREAM object, keeping all data and offsets intact */ void s_realloc(STREAM s, unsigned int size); +/* Free STREAM object and its associated buffer */ void s_free(STREAM s); +/* Reset all internal offsets, but keep the allocated size */ void s_reset(STREAM s); void out_utf16s(STREAM s, const char *string); diff --git a/tests/asn_test.c b/tests/asn_test.c index ff47d93..9546e59 100644 --- a/tests/asn_test.c +++ b/tests/asn_test.c @@ -46,23 +46,13 @@ xfree(void *mem) free(mem); } -static struct stream *stream_new(size_t size) { - struct stream *s; - s = malloc(sizeof(struct stream)); - memset(s, 0, sizeof(struct stream)); - s_realloc(s, size); - s_reset(s); - return(s); -} - - Ensure(ASN1, can_create_empty_sequence) { struct stream *s, *empty; uint8_t expected_data[] = {0x30, 0x00}; - s = stream_new(100); - empty = stream_new(100); + s = s_alloc(100); + empty = s_alloc(100); ber_out_sequence(s, empty); s_mark_end(s); @@ -76,7 +66,7 @@ Ensure(ASN1, can_create_empty_sequence_using_null) struct stream *s; uint8_t expected_data[] = {0x30, 0x00}; - s = stream_new(100); + s = s_alloc(100); ber_out_sequence(s, NULL); s_mark_end(s); @@ -90,8 +80,8 @@ Ensure(ASN1, can_create_sequence_of_two_integers) struct stream *s, *content; uint8_t expected_data[] = {0x30, 0x08, 0x02, 0x02, 0x00, 0xbe, 0x02, 0x02, 0x00, 0xef}; - s = stream_new(100); - content = stream_new(100); + s = s_alloc(100); + content = s_alloc(100); ber_out_integer(content, 0xbe); ber_out_integer(content, 0xef); @@ -109,8 +99,8 @@ Ensure(ASN1, can_create_sequence_of_one_integer) struct stream *s, *content; uint8_t expected_data[] = {0x30, 0x04, 0x02, 0x02, 0x00, 0xbe}; - s = stream_new(100); - content = stream_new(100); + s = s_alloc(100); + content = s_alloc(100); ber_out_integer(content, 0xbe); s_mark_end(content); diff --git a/tests/mcs_test.c b/tests/mcs_test.c index 37d6ff5..bad86be 100644 --- a/tests/mcs_test.c +++ b/tests/mcs_test.c @@ -52,15 +52,6 @@ xfree(void *mem) free(mem); } -static struct stream *stream_new(size_t size) { - struct stream *s; - s = malloc(sizeof(struct stream)); - memset(s, 0, sizeof(struct stream)); - s_realloc(s, size); - s_reset(s); - return(s); -} - /* Test function */ Ensure(MCS, should_produce_valid_packet_for_McsSendCJrq) @@ -69,7 +60,7 @@ Ensure(MCS, should_produce_valid_packet_for_McsSendCJrq) uint8_t content[] = {0x38, 0x00, 0x2A, 0x00, 0x0D}; struct stream *s; - s = stream_new(5); + s = s_alloc(5); chan_id = 13; g_mcs_userid = 42; @@ -89,7 +80,7 @@ Ensure(MCS, should_produce_valid_packet_for_McsSendDPU) struct stream *s; uint8_t content[] = {0x30, 0x06, 0x02, 0x02, 0x00, reason, 0x30, 0x00}; - s = stream_new(8); + s = s_alloc(8); expect(logger); expect(iso_init, will_return(s)); diff --git a/tests/resize_test.c b/tests/resize_test.c index 973d1fc..880e4c4 100644 --- a/tests/resize_test.c +++ b/tests/resize_test.c @@ -412,17 +412,6 @@ Ensure(Resize, UsingRDPEDISPHonoursServerSessionWidthConstraintMustBeEven) free(s.data); } -/* FIXME: promote to actual function in stream.c */ -STREAM s_alloc(size_t capacity) -{ - STREAM s; - s = xmalloc(sizeof(struct stream)); - memset(s, 0, sizeof(struct stream)); - s_realloc(s, capacity); - s_reset(s); - return s; -} - void get_width_and_height_from_mcs_connect_initial(int *width, int *height) { STREAM s; From 6268b44f0681995562f9e29f1f21b13a96ea6e14 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 13:13:35 +0200 Subject: [PATCH 13/21] Add macro to check remaining length of STREAM Avoids poking around in the internals, and also makes the code easier to read. --- channels.c | 4 ++-- iso.c | 2 +- lspci.c | 2 +- mcs.c | 2 +- rdp.c | 2 +- rdpdr.c | 2 +- rdpsnd.c | 8 ++++---- rdpsnd_alsa.c | 2 +- rdpsnd_libao.c | 2 +- rdpsnd_oss.c | 2 +- rdpsnd_pulse.c | 2 +- rdpsnd_sgi.c | 2 +- rdpsnd_sun.c | 2 +- seamless.c | 2 +- secure.c | 8 ++++---- stream.h | 4 +++- 16 files changed, 25 insertions(+), 23 deletions(-) diff --git a/channels.c b/channels.c index 952d676..440ece3 100644 --- a/channels.c +++ b/channels.c @@ -93,7 +93,7 @@ channel_send(STREAM s, VCHANNEL * channel) /* first fragment sent in-place */ s_pop_layer(s, channel_hdr); - length = s->end - s->p - 8; + length = s_remaining(s) - 8; logger(Protocol, Debug, "channel_send(), channel = %d, length = %d", channel->mcs_id, length); @@ -186,7 +186,7 @@ channel_process(STREAM s, uint16 mcs_channel) in->p = in->data; } - thislength = MIN(s->end - s->p, in->data + in->size - in->p); + thislength = MIN(s_remaining(s), in->data + in->size - in->p); memcpy(in->p, s->p, thislength); in->p += thislength; diff --git a/iso.c b/iso.c index a92c97b..bddfcd6 100644 --- a/iso.c +++ b/iso.c @@ -177,7 +177,7 @@ iso_send(STREAM s) uint16 length; s_pop_layer(s, iso_hdr); - length = s->end - s->p; + length = s_remaining(s); out_uint8(s, T123_HEADER_VERSION); /* version */ out_uint8(s, 0); /* reserved */ diff --git a/lspci.c b/lspci.c index 7690a1f..6b0f4d2 100644 --- a/lspci.c +++ b/lspci.c @@ -136,7 +136,7 @@ lspci_process(STREAM s) static char *rest = NULL; char *buf; - pkglen = s->end - s->p; + pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = xmalloc(pkglen + 1); STRNCPY(buf, (char *) s->p, pkglen + 1); diff --git a/mcs.c b/mcs.c index 67ec178..de6ab04 100644 --- a/mcs.c +++ b/mcs.c @@ -306,7 +306,7 @@ mcs_send_to_channel(STREAM s, uint16 channel) uint16 length; s_pop_layer(s, mcs_hdr); - length = s->end - s->p - 8; + length = s_remaining(s) - 8; length |= 0x8000; out_uint8(s, (MCS_SDRQ << 2)); diff --git a/rdp.c b/rdp.c index 2b12a01..ff8ccb3 100644 --- a/rdp.c +++ b/rdp.c @@ -194,7 +194,7 @@ rdp_send_data(STREAM s, uint8 data_pdu_type) uint16 length; s_pop_layer(s, rdp_hdr); - length = s->end - s->p; + length = s_remaining(s); out_uint16_le(s, length); out_uint16_le(s, (RDP_PDU_DATA | 0x10)); diff --git a/rdpdr.c b/rdpdr.c index ac6ecff..14b895e 100644 --- a/rdpdr.c +++ b/rdpdr.c @@ -879,7 +879,7 @@ rdpdr_process(STREAM s) uint16 pakid; logger(Protocol, Debug, "rdpdr_process()"); - /* hexdump(s->p, s->end - s->p); */ + /* hexdump(s->p, s_remaining(s)); */ in_uint16(s, component); /* RDPDR_HEADER.Component */ in_uint16(s, pakid); /* RDPDR_HEADER.PacketId */ diff --git a/rdpsnd.c b/rdpsnd.c index 9289380..dceaa08 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -343,7 +343,7 @@ rdpsnd_process_packet(uint8 opcode, STREAM s) } rdpsnd_queue_write(rdpsnd_dsp_process - (s->p, s->end - s->p, current_driver, + (s->p, s_remaining(s), current_driver, &formats[current_format]), tick, packet_index); return; break; @@ -386,7 +386,7 @@ rdpsnd_process(STREAM s) /* New packet */ if (packet.size == 0) { - if ((s->end - s->p) < 4) + if (!s_check_rem(s, 4)) { logger(Sound, Error, "rdpsnd_process(), split at packet header, things will go south from here..."); @@ -405,7 +405,7 @@ rdpsnd_process(STREAM s) } else { - len = MIN(s->end - s->p, packet.end - packet.p); + len = MIN(s_remaining(s), s_remaining(&packet)); /* Microsoft's server is so broken it's not even funny... */ if (packet_opcode == SNDC_WAVE) @@ -451,7 +451,7 @@ rdpsnddbg_process(STREAM s) static char *rest = NULL; char *buf; - pkglen = s->end - s->p; + pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = (char *) xmalloc(pkglen + 1); STRNCPY(buf, (char *) s->p, pkglen + 1); diff --git a/rdpsnd_alsa.c b/rdpsnd_alsa.c index f68562a..26e5cc8 100644 --- a/rdpsnd_alsa.c +++ b/rdpsnd_alsa.c @@ -380,7 +380,7 @@ alsa_play(void) next_tick = rdpsnd_queue_next_tick(); - len = (out->end - out->p) / (samplewidth_out * audiochannels_out); + len = s_remaining(out) / (samplewidth_out * audiochannels_out); if ((len = snd_pcm_writei(out_handle, out->p, ((MAX_FRAMES < len) ? MAX_FRAMES : len))) < 0) { snd_pcm_prepare(out_handle); diff --git a/rdpsnd_libao.c b/rdpsnd_libao.c index ddf2eaf..6d1996f 100644 --- a/rdpsnd_libao.c +++ b/rdpsnd_libao.c @@ -171,7 +171,7 @@ libao_play(void) next_tick = rdpsnd_queue_next_tick(); - len = (WAVEOUTLEN > (out->end - out->p)) ? (out->end - out->p) : WAVEOUTLEN; + len = MIN(WAVEOUTLEN, s_remaining(out)); ao_play(o_device, (char *) out->p, len); out->p += len; diff --git a/rdpsnd_oss.c b/rdpsnd_oss.c index 98e7e70..ae5df21 100644 --- a/rdpsnd_oss.c +++ b/rdpsnd_oss.c @@ -412,7 +412,7 @@ oss_play(void) packet = rdpsnd_queue_current_packet(); out = &packet->s; - len = out->end - out->p; + len = s_remaining(out); len = write(dsp_fd, out->p, (len > MAX_LEN) ? MAX_LEN : len); if (len == -1) diff --git a/rdpsnd_pulse.c b/rdpsnd_pulse.c index 413dba4..9bcee39 100644 --- a/rdpsnd_pulse.c +++ b/rdpsnd_pulse.c @@ -1192,7 +1192,7 @@ pulse_play(void) playback_seek = PA_SEEK_RELATIVE; avail_space = pa_stream_writable_size(playback_stream); - audio_size = out->end - out->p <= avail_space ? out->end - out->p : avail_space; + audio_size = MIN(s_remaining(out), avail_space); if (audio_size) { if (pa_stream_write diff --git a/rdpsnd_sgi.c b/rdpsnd_sgi.c index 07c07fd..d94f648 100644 --- a/rdpsnd_sgi.c +++ b/rdpsnd_sgi.c @@ -256,7 +256,7 @@ sgi_play(void) packet = rdpsnd_queue_current_packet(); out = (STREAM) (void *) &(packet->s); - len = out->end - out->p; + len = s_remaining(out); alWriteFrames(output_port, out->p, len / combinedFrameSize); diff --git a/rdpsnd_sun.c b/rdpsnd_sun.c index 3f78e24..50a8fb2 100644 --- a/rdpsnd_sun.c +++ b/rdpsnd_sun.c @@ -419,7 +419,7 @@ sun_play(void) packet = rdpsnd_queue_current_packet(); out = &packet->s; - len = out->end - out->p; + len = s_remaining(out); len = write(dsp_fd, out->p, (len > MAX_LEN) ? MAX_LEN : len); if (len == -1) diff --git a/seamless.c b/seamless.c index 0f294cc..d105a54 100644 --- a/seamless.c +++ b/seamless.c @@ -377,7 +377,7 @@ seamless_process(STREAM s) unsigned int pkglen; char *buf; - pkglen = s->end - s->p; + pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = xmalloc(pkglen + 1); STRNCPY(buf, (char *) s->p, pkglen + 1); diff --git a/secure.c b/secure.c index 980d5b2..59918a1 100644 --- a/secure.c +++ b/secure.c @@ -352,7 +352,7 @@ sec_send_to_channel(STREAM s, uint32 flags, uint16 channel) if (flags & SEC_ENCRYPT) { flags &= ~SEC_ENCRYPT; - datalen = s->end - s->p - 8; + datalen = s_remaining(s) - 8; sec_sign(s->p, 8, g_sec_sign_key, g_rc4_key_len, s->p + 8, datalen); sec_encrypt(s->p + 8, datalen); } @@ -885,7 +885,7 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s->end - s->p); + sec_decrypt(s->p, s_remaining(s)); } return s; } @@ -905,7 +905,7 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s->end - s->p); + sec_decrypt(s->p, s_remaining(s)); } if (sec_flags & SEC_LICENSE_PKT) @@ -923,7 +923,7 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s->end - s->p); + sec_decrypt(s->p, s_remaining(s)); /* Check for a redirect packet, starts with 00 04 */ if (s->p[0] == 0 && s->p[1] == 4) diff --git a/stream.h b/stream.h index ec61f0e..3005ad3 100644 --- a/stream.h +++ b/stream.h @@ -60,7 +60,9 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define s_push_layer(s,h,n) { (s)->h = (s)->p; (s)->p += n; } #define s_pop_layer(s,h) (s)->p = (s)->h; #define s_mark_end(s) (s)->end = (s)->p; -#define s_check_rem(s,n) (((s)->p <= (s)->end) && ((size_t)n <= (size_t)((s)->end - (s)->p))) +/* Returns number of bytes that can still be read from STREAM */ +#define s_remaining(s) (size_t)((s)->end - (s)->p) +#define s_check_rem(s,n) (((s)->p <= (s)->end) && ((size_t)n <= s_remaining(s))) #define s_check_end(s) ((s)->p == (s)->end) #define s_length(s) ((s)->end - (s)->data) #define s_left(s) ((s)->size - (size_t)((s)->p - (s)->data)) From 75221eb3c52a160fc8737bcb47b487d2df1579ff Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 Apr 2019 16:29:35 +0200 Subject: [PATCH 14/21] Separate behaviour of out_uint8a and out_uint8p Make them more match in_uint8a and in_uint8p in that one copies and the other just gives you a pointer and it is up to the caller how to fill it in. This can be useful when other APIs are used to generate the data as it avoids a temporary buffer. --- channels.c | 2 +- cliprdr.c | 2 +- cssp.c | 48 ++++++++++++++++++++++++------------------------ iso.c | 4 ++-- licence.c | 24 ++++++++++++------------ lspci.c | 2 +- mcs.c | 2 +- rdp.c | 6 +++--- rdp5.c | 2 +- rdpdr.c | 6 +++--- scard.c | 18 +++++++++--------- seamless.c | 2 +- secure.c | 2 +- stream.h | 10 +++++++--- 14 files changed, 67 insertions(+), 63 deletions(-) diff --git a/channels.c b/channels.c index 440ece3..5fa6923 100644 --- a/channels.c +++ b/channels.c @@ -134,7 +134,7 @@ channel_send(STREAM s, VCHANNEL * channel) s = sec_init(g_encryption ? SEC_ENCRYPT : 0, thislength + 8); out_uint32_le(s, length); out_uint32_le(s, flags); - out_uint8p(s, data, thislength); + out_uint8a(s, data, thislength); s_mark_end(s); sec_send_to_channel(s, g_encryption ? SEC_ENCRYPT : 0, channel->mcs_id); diff --git a/cliprdr.c b/cliprdr.c index 337b91b..0ee9446 100644 --- a/cliprdr.c +++ b/cliprdr.c @@ -48,7 +48,7 @@ cliprdr_send_packet(uint16 type, uint16 status, uint8 * data, uint32 length) out_uint16_le(s, type); out_uint16_le(s, status); out_uint32_le(s, length); - out_uint8p(s, data, length); + out_uint8a(s, data, length); out_uint32(s, 0); /* pad? */ s_mark_end(s); channel_send(s, cliprdr_channel); diff --git a/cssp.c b/cssp.c index 98237d8..ee19c9e 100644 --- a/cssp.c +++ b/cssp.c @@ -38,7 +38,7 @@ ber_wrap_hdr_data(int tagval, STREAM in) out = s_alloc(size); ber_out_header(out, tagval, s_length(in)); - out_uint8p(out, in->data, s_length(in)); + out_uint8a(out, in->data, s_length(in)); s_mark_end(out); return out; @@ -170,7 +170,7 @@ cssp_gss_wrap(gss_ctx_id_t ctx, STREAM in, STREAM out) // write enc data to out stream out->data = out->p = xmalloc(outbuf.length); out->size = outbuf.length; - out_uint8p(out, outbuf.value, outbuf.length); + out_uint8a(out, outbuf.value, outbuf.length); s_mark_end(out); gss_release_buffer(&minor_status, &outbuf); @@ -201,7 +201,7 @@ cssp_gss_unwrap(gss_ctx_id_t ctx, STREAM in, STREAM out) out->data = out->p = xmalloc(outbuf.length); out->size = outbuf.length; - out_uint8p(out, outbuf.value, outbuf.length); + out_uint8a(out, outbuf.value, outbuf.length); s_mark_end(out); gss_release_buffer(&minor_status, &outbuf); @@ -229,7 +229,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -242,7 +242,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -254,7 +254,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -290,7 +290,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -304,7 +304,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -319,7 +319,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -334,7 +334,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -349,7 +349,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 4, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -382,7 +382,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -392,7 +392,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) g_sc_container_name, g_sc_csp_name); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -406,7 +406,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -421,7 +421,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -462,7 +462,7 @@ cssp_encode_tscredentials(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -480,7 +480,7 @@ cssp_encode_tscredentials(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, h3); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h3); s_free(h2); @@ -516,7 +516,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -530,7 +530,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h2 = ber_wrap_hdr_data(BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED, h3); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h5); s_free(h4); @@ -546,7 +546,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_free(h2); s_free(h1); @@ -559,7 +559,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8p(&message, h1->data, s_length(h1)); + out_uint8a(&message, h1->data, s_length(h1)); s_mark_end(&message); s_free(h2); s_free(h1); @@ -570,7 +570,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) // Todo: can h1 be send directly instead of tcp_init() approach h1 = ber_wrap_hdr_data(BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED, &message); s = tcp_init(s_length(h1)); - out_uint8p(s, h1->data, s_length(h1)); + out_uint8a(s, h1->data, s_length(h1)); s_mark_end(s); s_free(h1); @@ -664,7 +664,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) s_realloc(token, length); s_reset(token); - out_uint8p(token, s->p, length); + out_uint8a(token, s->p, length); s_mark_end(token); } @@ -785,7 +785,7 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) s_realloc(&token, output_tok.length); s_reset(&token); - out_uint8p(&token, output_tok.value, output_tok.length); + out_uint8a(&token, output_tok.value, output_tok.length); s_mark_end(&token); if (!cssp_send_tsrequest(&token, NULL, NULL)) diff --git a/iso.c b/iso.c index bddfcd6..2377a1b 100644 --- a/iso.c +++ b/iso.c @@ -78,8 +78,8 @@ iso_send_connection_request(char *username, uint32 neg_proto) out_uint16(s, 0); /* src_ref */ out_uint8(s, 0); /* class */ - out_uint8p(s, "Cookie: mstshash=", strlen("Cookie: mstshash=")); - out_uint8p(s, username, strlen(username)); + out_uint8a(s, "Cookie: mstshash=", strlen("Cookie: mstshash=")); + out_uint8a(s, username, strlen(username)); out_uint8(s, 0x0d); /* cookie termination string: CR+LF */ out_uint8(s, 0x0a); diff --git a/licence.c b/licence.c index 908b7f2..ec74b09 100644 --- a/licence.c +++ b/licence.c @@ -79,21 +79,21 @@ licence_info(uint8 * client_random, uint8 * rsa_data, out_uint16(s, 0); out_uint16_le(s, 0x0201); - out_uint8p(s, client_random, SEC_RANDOM_SIZE); + out_uint8a(s, client_random, SEC_RANDOM_SIZE); out_uint16_le(s, 2); out_uint16_le(s, (SEC_MODULUS_SIZE + SEC_PADDING_SIZE)); - out_uint8p(s, rsa_data, SEC_MODULUS_SIZE); + out_uint8a(s, rsa_data, SEC_MODULUS_SIZE); out_uint8s(s, SEC_PADDING_SIZE); out_uint16_le(s, 1); out_uint16_le(s, licence_size); - out_uint8p(s, licence_data, licence_size); + out_uint8a(s, licence_data, licence_size); out_uint16_le(s, 1); out_uint16_le(s, LICENCE_HWID_SIZE); - out_uint8p(s, hwid, LICENCE_HWID_SIZE); + out_uint8a(s, hwid, LICENCE_HWID_SIZE); - out_uint8p(s, signature, LICENCE_SIGNATURE_SIZE); + out_uint8a(s, signature, LICENCE_SIGNATURE_SIZE); s_mark_end(s); sec_send(s, sec_flags); @@ -120,21 +120,21 @@ licence_send_new_licence_request(uint8 * client_random, uint8 * rsa_data, char * out_uint16(s, 0); out_uint16_le(s, 0xff01); - out_uint8p(s, client_random, SEC_RANDOM_SIZE); + out_uint8a(s, client_random, SEC_RANDOM_SIZE); out_uint16_le(s, 2); out_uint16_le(s, (SEC_MODULUS_SIZE + SEC_PADDING_SIZE)); - out_uint8p(s, rsa_data, SEC_MODULUS_SIZE); + out_uint8a(s, rsa_data, SEC_MODULUS_SIZE); out_uint8s(s, SEC_PADDING_SIZE); /* Username LICENSE_BINARY_BLOB */ out_uint16_le(s, BB_CLIENT_USER_NAME_BLOB); out_uint16_le(s, userlen); - out_uint8p(s, user, userlen); + out_uint8a(s, user, userlen); /* Machinename LICENSE_BINARY_BLOB */ out_uint16_le(s, BB_CLIENT_MACHINE_NAME_BLOB); out_uint16_le(s, hostlen); - out_uint8p(s, host, hostlen); + out_uint8a(s, host, hostlen); s_mark_end(s); sec_send(s, sec_flags); @@ -204,13 +204,13 @@ licence_send_platform_challenge_response(uint8 * token, uint8 * crypt_hwid, uint out_uint16_le(s, 1); out_uint16_le(s, LICENCE_TOKEN_SIZE); - out_uint8p(s, token, LICENCE_TOKEN_SIZE); + out_uint8a(s, token, LICENCE_TOKEN_SIZE); out_uint16_le(s, 1); out_uint16_le(s, LICENCE_HWID_SIZE); - out_uint8p(s, crypt_hwid, LICENCE_HWID_SIZE); + out_uint8a(s, crypt_hwid, LICENCE_HWID_SIZE); - out_uint8p(s, signature, LICENCE_SIGNATURE_SIZE); + out_uint8a(s, signature, LICENCE_SIGNATURE_SIZE); s_mark_end(s); sec_send(s, sec_flags); diff --git a/lspci.c b/lspci.c index 6b0f4d2..05d92ba 100644 --- a/lspci.c +++ b/lspci.c @@ -163,6 +163,6 @@ lspci_send(const char *output) len = strlen(output); s = channel_init(lspci_channel, len); - out_uint8p(s, output, len) s_mark_end(s); + out_uint8a(s, output, len) s_mark_end(s); channel_send(s, lspci_channel); } diff --git a/mcs.c b/mcs.c index de6ab04..3904430 100644 --- a/mcs.c +++ b/mcs.c @@ -82,7 +82,7 @@ mcs_send_connect_initial(STREAM mcs_data) mcs_out_domain_params(s, 0xffff, 0xfc17, 0xffff, 0xffff); /* max params */ ber_out_header(s, BER_TAG_OCTET_STRING, datalen); - out_uint8p(s, mcs_data->data, datalen); + out_uint8a(s, mcs_data->data, datalen); s_mark_end(s); iso_send(s); diff --git a/rdp.c b/rdp.c index ff8ccb3..8ef0ee7 100644 --- a/rdp.c +++ b/rdp.c @@ -469,7 +469,7 @@ rdp_send_client_info_pdu(uint32 flags, char *domain, char *user, if (g_redirect == True && 0 < g_redirect_cookie_len) { - out_uint8p(s, g_redirect_cookie, g_redirect_cookie_len); + out_uint8a(s, g_redirect_cookie, g_redirect_cookie_len); } else { @@ -792,7 +792,7 @@ rdp_out_ts_order_capabilityset(STREAM s) out_uint16_le(s, 1); /* maximumOrderLevel (ignored, should be 1) */ out_uint16_le(s, 0); /* numberFonts (ignored, should be 0) */ out_uint16_le(s, orderflags); /* orderFlags */ - out_uint8p(s, order_caps, 32); /* orderSupport */ + out_uint8a(s, order_caps, 32); /* orderSupport */ out_uint16_le(s, 0); /* textFlags (ignored) */ out_uint16_le(s, 0); /* orderSupportExFlags */ out_uint32_le(s, 0); /* pad4OctetsB */ @@ -1096,7 +1096,7 @@ rdp_send_confirm_active(void) out_uint16_le(s, sizeof(RDP_SOURCE)); out_uint16_le(s, caplen); - out_uint8p(s, RDP_SOURCE, sizeof(RDP_SOURCE)); + out_uint8a(s, RDP_SOURCE, sizeof(RDP_SOURCE)); out_uint16_le(s, 17); /* num_caps */ out_uint8s(s, 2); /* pad */ diff --git a/rdp5.c b/rdp5.c index 9e8790b..5a8de96 100644 --- a/rdp5.c +++ b/rdp5.c @@ -140,7 +140,7 @@ process_ts_fp_updates(STREAM s) s_reset(assembled[code]); } - out_uint8p(assembled[code], ts->p, length); + out_uint8a(assembled[code], ts->p, length); if (frag == FASTPATH_FRAGMENT_LAST) { diff --git a/rdpdr.c b/rdpdr.c index 14b895e..3a5f1f7 100644 --- a/rdpdr.c +++ b/rdpdr.c @@ -300,7 +300,7 @@ rdpdr_send_client_device_list_announce(void) { out_uint32_le(s, g_rdpdr_device[i].device_type); out_uint32_le(s, i); /* RDP Device ID */ - out_uint8p(s, g_rdpdr_device[i].name, 8); /* preferredDosName, limited to 8 characters */ + out_uint8a(s, g_rdpdr_device[i].name, 8); /* preferredDosName, limited to 8 characters */ switch (g_rdpdr_device[i].device_type) { case DEVICE_TYPE_DISK: @@ -314,7 +314,7 @@ rdpdr_send_client_device_list_announce(void) disklen = strlen(diskinfo->name) + 1; out_uint32_le(s, disklen); /* DeviceDataLength */ - out_uint8p(s, diskinfo->name, disklen); /* DeviceData */ + out_uint8a(s, diskinfo->name, disklen); /* DeviceData */ break; case DEVICE_TYPE_PRINTER: @@ -385,7 +385,7 @@ rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, ui out_uint32_le(s, status); out_uint32_le(s, result); if (length) - out_uint8p(s, buffer, length); + out_uint8a(s, buffer, length); s_mark_end(s); logger(Protocol, Debug, "rdpdr_send_completion()"); diff --git a/scard.c b/scard.c index 52d94b9..9e4e807 100644 --- a/scard.c +++ b/scard.c @@ -548,7 +548,7 @@ outBufferFinishWithLimit(STREAM out, char *buffer, unsigned int length, unsigned { if (header < length) length = header; - out_uint8p(out, buffer, length); + out_uint8a(out, buffer, length); outRepos(out, length); } } @@ -624,11 +624,11 @@ outString(STREAM out, char *source, RD_BOOL wide) buffer[2 * i] = reader[i]; buffer[2 * i + 1] = '\0'; } - out_uint8p(out, buffer, 2 * dataLength); + out_uint8a(out, buffer, 2 * dataLength); } else { - out_uint8p(out, reader, dataLength); + out_uint8a(out, reader, dataLength); } SC_xfreeallmemory(&lcHandle); @@ -1255,7 +1255,7 @@ TS_SCardGetStatusChange(STREAM in, STREAM out, RD_BOOL wide) cur->dwEventState = swap32(cur->dwEventState); cur->cbAtr = swap32(cur->cbAtr); - out_uint8p(out, (void *) ((unsigned char **) cur + 2), + out_uint8a(out, (void *) ((unsigned char **) cur + 2), sizeof(SERVER_SCARD_READERSTATE_A) - 2 * sizeof(unsigned char *)); } outForceAlignment(out, 8); @@ -1419,7 +1419,7 @@ TS_SCardLocateCardsByATR(STREAM in, STREAM out, RD_BOOL wide) rsCur->dwEventState = swap32(rsCur->dwEventState); rsCur->cbAtr = swap32(rsCur->cbAtr); - out_uint8p(out, (void *) ((unsigned char **) rsCur + 2), + out_uint8a(out, (void *) ((unsigned char **) rsCur + 2), sizeof(SCARD_READERSTATE) - 2 * sizeof(unsigned char *)); } @@ -1795,7 +1795,7 @@ TS_SCardStatus(STREAM in, STREAM out, RD_BOOL wide) out_uint32_le(out, 0x00020000); out_uint32_le(out, dwState); out_uint32_le(out, dwProtocol); - out_uint8p(out, atr, dwAtrLen); + out_uint8a(out, atr, dwAtrLen); if (dwAtrLen < 32) { out_uint8s(out, 32 - dwAtrLen); @@ -1901,7 +1901,7 @@ TS_SCardState(STREAM in, STREAM out) out_uint32_le(out, dwAtrLen); out_uint32_le(out, 0x00000001); out_uint32_le(out, dwAtrLen); - out_uint8p(out, atr, dwAtrLen); + out_uint8a(out, atr, dwAtrLen); outRepos(out, dwAtrLen); } outForceAlignment(out, 8); @@ -2038,7 +2038,7 @@ TS_SCardGetAttrib(STREAM in, STREAM out) } else { - out_uint8p(out, pbAttr, dwAttrLen); + out_uint8a(out, pbAttr, dwAttrLen); } outRepos(out, dwAttrLen); out_uint32_le(out, 0x00000000); @@ -2236,7 +2236,7 @@ TS_SCardControl(STREAM in, STREAM out) out_uint32_le(out, nBytesReturned); if (nBytesReturned > 0) { - out_uint8p(out, pOutBuffer, nBytesReturned); + out_uint8a(out, pOutBuffer, nBytesReturned); outRepos(out, nBytesReturned); } diff --git a/seamless.c b/seamless.c index d105a54..05782d9 100644 --- a/seamless.c +++ b/seamless.c @@ -440,7 +440,7 @@ seamless_send(const char *command, const char *format, ...) len++; s = channel_init(seamless_channel, len); - out_uint8p(s, buf, len) s_mark_end(s); + out_uint8a(s, buf, len) s_mark_end(s); logger(Core, Debug, "seamless_send(), sending '%s'", buf); diff --git a/secure.c b/secure.c index 59918a1..1453a90 100644 --- a/secure.c +++ b/secure.c @@ -384,7 +384,7 @@ sec_establish_key(void) s = sec_init(flags, length + 4); out_uint32_le(s, length); - out_uint8p(s, g_sec_crypted_random, g_server_public_key_len); + out_uint8a(s, g_sec_crypted_random, g_server_public_key_len); out_uint8s(s, SEC_PADDING_SIZE); s_mark_end(s); diff --git a/stream.h b/stream.h index 3005ad3..45462e0 100644 --- a/stream.h +++ b/stream.h @@ -127,14 +127,18 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #endif #define in_uint8(s,v) { s_assert_r(s, 1); v = *((s)->p++); } +/* Return a pointer in v to manually read n bytes from STREAM s */ #define in_uint8p(s,v,n) { s_assert_r(s, n); v = (s)->p; (s)->p += n; } +/* Copy n bytes from STREAM s in to array v */ #define in_uint8a(s,v,n) { s_assert_r(s, n); memcpy(v,(s)->p,n); (s)->p += n; } #define in_uint8s(s,n) { s_assert_r(s, n); (s)->p += n; } #define out_uint8(s,v) { s_assert_w(s, 1); *((s)->p++) = v; } -#define out_uint8p(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } -#define out_uint8a(s,v,n) out_uint8p(s,v,n); +/* Return a pointer in v to manually fill in n bytes in STREAM s */ +#define out_uint8p(s,v,n) { s_assert_w(s, n); v = (s)->p; (s)->p += n; } +/* Copy n bytes from array v in to STREAM s */ +#define out_uint8a(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } #define out_uint8s(s,n) { s_assert_w(s, n); memset((s)->p,0,n); (s)->p += n; } -#define out_stream(s, v) out_uint8p(s, (v)->data, s_length((v))) +#define out_stream(s, v) out_uint8a(s, (v)->data, s_length((v))) #define next_be(s,v) { s_assert_r(s, 1); v = ((v) << 8) + *((s)->p++); } From c6d8b933c85f71ec27d8566904615c6ef2438b45 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 12 Apr 2019 14:22:35 +0200 Subject: [PATCH 15/21] Avoid preallocated streams in tcp.c We don't know when the caller might be done with a stream, so we can end up with code overwriting things in a stream that is in use elsewhere. Solve the issue by returning a new stream each time and leave it up to the callers to free it. --- channels.c | 1 + cliprdr.c | 1 + cssp.c | 1 + dvc.c | 3 +++ iso.c | 2 ++ licence.c | 3 +++ lspci.c | 1 + mcs.c | 5 +++++ rdp.c | 8 ++++++++ rdpdr.c | 5 +++++ rdpsnd.c | 3 +++ seamless.c | 1 + secure.c | 1 + tcp.c | 46 +--------------------------------------------- 14 files changed, 36 insertions(+), 45 deletions(-) diff --git a/channels.c b/channels.c index 5fa6923..d76fd1c 100644 --- a/channels.c +++ b/channels.c @@ -137,6 +137,7 @@ channel_send(STREAM s, VCHANNEL * channel) out_uint8a(s, data, thislength); s_mark_end(s); sec_send_to_channel(s, g_encryption ? SEC_ENCRYPT : 0, channel->mcs_id); + s_free(s); data += thislength; } diff --git a/cliprdr.c b/cliprdr.c index 0ee9446..c05e281 100644 --- a/cliprdr.c +++ b/cliprdr.c @@ -52,6 +52,7 @@ cliprdr_send_packet(uint16 type, uint16 status, uint8 * data, uint32 length) out_uint32(s, 0); /* pad? */ s_mark_end(s); channel_send(s, cliprdr_channel); + s_free(s); } /* Helper which announces our readiness to supply clipboard data diff --git a/cssp.c b/cssp.c index ee19c9e..373ed25 100644 --- a/cssp.c +++ b/cssp.c @@ -575,6 +575,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) s_free(h1); tcp_send(s); + s_free(s); // cleanup xfree(message.data); diff --git a/dvc.c b/dvc.c index c75bad1..cb0f95a 100644 --- a/dvc.c +++ b/dvc.c @@ -268,6 +268,7 @@ dvc_send(const char *name, STREAM s) s_mark_end(ls); channel_send(ls, dvc_channel); + s_free(ls); } @@ -292,6 +293,7 @@ dvc_send_capabilities_response() s_mark_end(s); channel_send(s, dvc_channel); + s_free(s); } static void @@ -320,6 +322,7 @@ dvc_send_create_response(RD_BOOL success, dvc_hdr_t hdr, uint32 channelid) s_mark_end(s); channel_send(s, dvc_channel); + s_free(s); } static void diff --git a/iso.c b/iso.c index 2377a1b..115f810 100644 --- a/iso.c +++ b/iso.c @@ -55,6 +55,7 @@ iso_send_msg(uint8 code) s_mark_end(s); tcp_send(s); + s_free(s); } static void @@ -95,6 +96,7 @@ iso_send_connection_request(char *username, uint32 neg_proto) s_mark_end(s); tcp_send(s); + s_free(s); } /* Receive a message on the ISO layer, return code */ diff --git a/licence.c b/licence.c index ec74b09..2125254 100644 --- a/licence.c +++ b/licence.c @@ -97,6 +97,7 @@ licence_info(uint8 * client_random, uint8 * rsa_data, s_mark_end(s); sec_send(s, sec_flags); + s_free(s); } /* Send a new licence request packet */ @@ -138,6 +139,7 @@ licence_send_new_licence_request(uint8 * client_random, uint8 * rsa_data, char * s_mark_end(s); sec_send(s, sec_flags); + s_free(s); } /* Process a licence request packet */ @@ -214,6 +216,7 @@ licence_send_platform_challenge_response(uint8 * token, uint8 * crypt_hwid, uint s_mark_end(s); sec_send(s, sec_flags); + s_free(s); } /* Parse an platform challenge request packet */ diff --git a/lspci.c b/lspci.c index 05d92ba..73b974d 100644 --- a/lspci.c +++ b/lspci.c @@ -165,4 +165,5 @@ lspci_send(const char *output) s = channel_init(lspci_channel, len); out_uint8a(s, output, len) s_mark_end(s); channel_send(s, lspci_channel); + s_free(s); } diff --git a/mcs.c b/mcs.c index 3904430..a13092b 100644 --- a/mcs.c +++ b/mcs.c @@ -86,6 +86,7 @@ mcs_send_connect_initial(STREAM mcs_data) s_mark_end(s); iso_send(s); + s_free(s); } /* Expect a MCS_CONNECT_RESPONSE message (ASN.1 BER) */ @@ -159,6 +160,7 @@ mcs_send_edrq(void) s_mark_end(s); iso_send(s); + s_free(s); } /* Send an AUrq message (ASN.1 PER) */ @@ -173,6 +175,7 @@ mcs_send_aurq(void) s_mark_end(s); iso_send(s); + s_free(s); } /* Expect a AUcf message (ASN.1 PER) */ @@ -226,6 +229,7 @@ mcs_send_cjrq(uint16 chanid) s_mark_end(s); iso_send(s); + s_free(s); } /* Expect a CJcf message (ASN.1 PER) */ @@ -285,6 +289,7 @@ mcs_send_dpu(unsigned short reason) s_mark_end(s); iso_send(s); + s_free(s); } /* Initialise an MCS transport data packet */ diff --git a/rdp.c b/rdp.c index 8ef0ee7..ffc8920 100644 --- a/rdp.c +++ b/rdp.c @@ -535,6 +535,7 @@ rdp_send_client_info_pdu(uint32 flags, char *domain, char *user, g_redirect = False; sec_send(s, sec_flags); + s_free(s); } /* Send a control PDU */ @@ -551,6 +552,7 @@ rdp_send_control(uint16 action) s_mark_end(s); rdp_send_data(s, RDP_DATA_PDU_CONTROL); + s_free(s); } /* Send a synchronisation PDU */ @@ -568,6 +570,7 @@ rdp_send_synchronise(void) s_mark_end(s); rdp_send_data(s, RDP_DATA_PDU_SYNCHRONISE); + s_free(s); } /* Send a single input event */ @@ -591,6 +594,7 @@ rdp_send_input(uint32 time, uint16 message_type, uint16 device_flags, uint16 par s_mark_end(s); rdp_send_data(s, RDP_DATA_PDU_INPUT); + s_free(s); } /* Send a Suppress Output PDU */ @@ -625,6 +629,7 @@ rdp_send_suppress_output_pdu(enum RDP_SUPPRESS_STATUS allowupdates) s_mark_end(s); rdp_send_data(s, RDP_DATA_PDU_CLIENT_WINDOW_STATUS); + s_free(s); current_status = allowupdates; } @@ -669,6 +674,7 @@ rdp_enum_bmpcache2(void) s_mark_end(s); rdp_send_data(s, 0x2b); + s_free(s); offset += 169; } @@ -691,6 +697,7 @@ rdp_send_fonts(uint16 seq) s_mark_end(s); rdp_send_data(s, RDP_DATA_PDU_FONT2); + s_free(s); } /* Output general capability set (TS_GENERAL_CAPABILITYSET) */ @@ -1129,6 +1136,7 @@ rdp_send_confirm_active(void) s_mark_end(s); sec_send(s, sec_flags); + s_free(s); } /* Process a general capability set */ diff --git a/rdpdr.c b/rdpdr.c index 3a5f1f7..0221366 100644 --- a/rdpdr.c +++ b/rdpdr.c @@ -204,6 +204,7 @@ rdpdr_send_client_announce_reply(void) out_uint32_be(s, g_client_id); /* ClientID */ s_mark_end(s); channel_send(s, rdpdr_channel); + s_free(s); } @@ -233,6 +234,7 @@ rdpdr_send_client_name_request(void) out_stream(s, &name); s_mark_end(s); channel_send(s, rdpdr_channel); + s_free(s); } /* Returns the size of the payload of the announce packet */ @@ -357,6 +359,7 @@ rdpdr_send_client_device_list_announce(void) s_mark_end(s); channel_send(s, rdpdr_channel); + s_free(s); } void @@ -392,6 +395,7 @@ rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, ui /* hexdump(s->channel_hdr + 8, s->end - s->channel_hdr - 8); */ channel_send(s, rdpdr_channel); + s_free(s); #ifdef WITH_SCARD scard_unlock(SCARD_LOCK_RDPDR); #endif @@ -868,6 +872,7 @@ rdpdr_send_client_capability_response(void) s_mark_end(s); channel_send(s, rdpdr_channel); + s_free(s); } static void diff --git a/rdpsnd.c b/rdpsnd.c index dceaa08..b5b8ded 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -103,6 +103,7 @@ rdpsnd_send_waveconfirm(uint16 tick, uint8 packet_index) out_uint8(s, 0); s_mark_end(s); rdpsnd_send(s); + s_free(s); logger(Sound, Debug, "rdpsnd_send_waveconfirm(), tick=%u, index=%u", (unsigned) tick, (unsigned) packet_index); @@ -259,6 +260,7 @@ rdpsnd_process_negotiate(STREAM in) (int) format_count); rdpsnd_send(out); + s_free(out); rdpsnd_negotiated = True; } @@ -286,6 +288,7 @@ rdpsnd_process_training(STREAM in) out_uint16_le(out, packsize); s_mark_end(out); rdpsnd_send(out); + s_free(out); } static void diff --git a/seamless.c b/seamless.c index 05782d9..f5cc7a4 100644 --- a/seamless.c +++ b/seamless.c @@ -445,6 +445,7 @@ seamless_send(const char *command, const char *format, ...) logger(Core, Debug, "seamless_send(), sending '%s'", buf); channel_send(s, seamless_channel); + s_free(s); return seamless_serial++; } diff --git a/secure.c b/secure.c index 1453a90..8c5dd42 100644 --- a/secure.c +++ b/secure.c @@ -389,6 +389,7 @@ sec_establish_key(void) s_mark_end(s); sec_send(s, flags); + s_free(s); } /* Output connect initial data blob */ diff --git a/tcp.c b/tcp.c index 69e74b9..726f6c0 100644 --- a/tcp.c +++ b/tcp.c @@ -58,12 +58,6 @@ #define INADDR_NONE ((unsigned long) -1) #endif -#ifdef WITH_SCARD -#define STREAM_COUNT 8 -#else -#define STREAM_COUNT 1 -#endif - #ifdef IPv6 static struct addrinfo *g_server_address = NULL; #else @@ -75,7 +69,6 @@ static RD_BOOL g_ssl_initialized = False; static int g_sock; static RD_BOOL g_run_ui = False; static struct stream g_in; -static struct stream g_out[STREAM_COUNT]; int g_tcp_port_rdp = TCP_PORT_RDP; extern RD_BOOL g_exit_mainloop; @@ -109,20 +102,7 @@ tcp_can_send(int sck, int millis) STREAM tcp_init(uint32 maxlen) { - static int cur_stream_id = 0; - STREAM result = NULL; - -#ifdef WITH_SCARD - scard_lock(SCARD_LOCK_TCP); -#endif - result = &g_out[cur_stream_id]; - s_realloc(result, maxlen); - s_reset(result); - cur_stream_id = (cur_stream_id + 1) % STREAM_COUNT; -#ifdef WITH_SCARD - scard_unlock(SCARD_LOCK_TCP); -#endif - return result; + return s_alloc(maxlen); } /* Send TCP transport data packet */ @@ -527,7 +507,6 @@ tcp_connect(char *server) { socklen_t option_len; uint32 option_value; - int i; char buf[NI_MAXHOST]; #ifdef IPv6 @@ -678,12 +657,6 @@ tcp_connect(char *server) g_in.size = 4096; g_in.data = (uint8 *) xmalloc(g_in.size); - for (i = 0; i < STREAM_COUNT; i++) - { - g_out[i].size = 4096; - g_out[i].data = (uint8 *) xmalloc(g_out[i].size); - } - /* After successful connect: update the last server name */ if (g_last_server_name) xfree(g_last_server_name); @@ -695,8 +668,6 @@ tcp_connect(char *server) void tcp_disconnect(void) { - int i; - if (g_ssl_initialized) { (void)gnutls_bye(g_tls_session, GNUTLS_SHUT_WR); gnutls_deinit(g_tls_session); @@ -712,13 +683,6 @@ tcp_disconnect(void) g_in.size = 0; xfree(g_in.data); g_in.data = NULL; - - for (i = 0; i < STREAM_COUNT; i++) - { - g_out[i].size = 0; - xfree(g_out[i].data); - g_out[i].data = NULL; - } } char * @@ -752,16 +716,8 @@ tcp_is_connected() void tcp_reset_state(void) { - int i; - /* Clear the incoming stream */ s_reset(&g_in); - - /* Clear the outgoing stream(s) */ - for (i = 0; i < STREAM_COUNT; i++) - { - s_reset(&g_out[i]); - } } void From 655c3d56dfd28037209d955363c198a3c818cefd Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 11 Apr 2019 13:05:38 +0200 Subject: [PATCH 16/21] Clean up channel chunk sending code Make sure the buffer handling is a bit more sane so we can verify offsets and boundaries. Also adds some more helper macros to shuffle data between two different STREAM instead of trying to poke around in the internals. --- channels.c | 131 +++++++++++++++++++++++++++++++++-------------------- stream.h | 6 +++ 2 files changed, 89 insertions(+), 48 deletions(-) diff --git a/channels.c b/channels.c index d76fd1c..b5ea690 100644 --- a/channels.c +++ b/channels.c @@ -80,66 +80,101 @@ channel_init(VCHANNEL * channel, uint32 length) return s; } -void -channel_send(STREAM s, VCHANNEL * channel) +static void +channel_send_chunk(STREAM s, VCHANNEL * channel, uint32 length) { - uint32 length, flags; - uint32 thislength, remaining; - uint8 *data; + uint32 flags; + uint32 thislength; + RD_BOOL inplace; + STREAM chunk; -#ifdef WITH_SCARD - scard_lock(SCARD_LOCK_CHANNEL); -#endif - - /* first fragment sent in-place */ - s_pop_layer(s, channel_hdr); - length = s_remaining(s) - 8; - - logger(Protocol, Debug, "channel_send(), channel = %d, length = %d", channel->mcs_id, - length); - - thislength = MIN(length, vc_chunk_size); -/* Note: In the original clipboard implementation, this number was - 1592, not 1600. However, I don't remember the reason and 1600 seems - to work so.. This applies only to *this* length, not the length of - continuation or ending packets. */ + /* Note: In the original clipboard implementation, this number was + 1592, not 1600. However, I don't remember the reason and 1600 seems + to work so.. This applies only to *this* length, not the length of + continuation or ending packets. */ /* Actually, CHANNEL_CHUNK_LENGTH (default value is 1600 bytes) is described in MS-RDPBCGR (s. 2.2.6, s.3.1.5.2.1) and can be set by server only in the optional field VCChunkSize of VC Caps) */ - remaining = length - thislength; - flags = (remaining == 0) ? CHANNEL_FLAG_FIRST | CHANNEL_FLAG_LAST : CHANNEL_FLAG_FIRST; - if (channel->flags & CHANNEL_OPTION_SHOW_PROTOCOL) - flags |= CHANNEL_FLAG_SHOW_PROTOCOL; + thislength = MIN(s_remaining(s), vc_chunk_size); - out_uint32_le(s, length); - out_uint32_le(s, flags); - data = s->end = s->p + thislength; - logger(Protocol, Debug, "channel_send(), sending %d bytes with FLAG_FIRST set", thislength); - sec_send_to_channel(s, g_encryption ? SEC_ENCRYPT : 0, channel->mcs_id); - - /* subsequent segments copied (otherwise would have to generate headers backwards) */ - while (remaining > 0) + flags = 0; + if (length == s_remaining(s)) { - thislength = MIN(remaining, vc_chunk_size); - remaining -= thislength; - flags = (remaining == 0) ? CHANNEL_FLAG_LAST : 0; - if (channel->flags & CHANNEL_OPTION_SHOW_PROTOCOL) - flags |= CHANNEL_FLAG_SHOW_PROTOCOL; + flags |= CHANNEL_FLAG_FIRST; + } + if (s_remaining(s) == thislength) + { + flags |= CHANNEL_FLAG_LAST; + } + if (channel->flags & CHANNEL_OPTION_SHOW_PROTOCOL) + { + flags |= CHANNEL_FLAG_SHOW_PROTOCOL; + } - logger(Protocol, Debug, "channel_send(), sending %d bytes with flags 0x%x", - thislength, flags); + logger(Protocol, Debug, "channel_send_chunk(), sending %d bytes with flags 0x%x", + thislength, flags); - s = sec_init(g_encryption ? SEC_ENCRYPT : 0, thislength + 8); - out_uint32_le(s, length); - out_uint32_le(s, flags); - out_uint8a(s, data, thislength); - s_mark_end(s); - sec_send_to_channel(s, g_encryption ? SEC_ENCRYPT : 0, channel->mcs_id); - s_free(s); + /* first fragment sent in-place */ + inplace = False; + if ((flags & (CHANNEL_FLAG_FIRST|CHANNEL_FLAG_LAST)) == + (CHANNEL_FLAG_FIRST|CHANNEL_FLAG_LAST)) + { + inplace = True; + } - data += thislength; + if (inplace) + { + s_pop_layer(s, channel_hdr); + chunk = s; + } + else + { + chunk = sec_init(g_encryption ? SEC_ENCRYPT : 0, thislength + 8); + } + + out_uint32_le(chunk, length); + out_uint32_le(chunk, flags); + if (!inplace) + { + out_uint8stream(chunk, s, thislength); + s_mark_end(chunk); + } + sec_send_to_channel(chunk, g_encryption ? SEC_ENCRYPT : 0, channel->mcs_id); + + /* Sending modifies the current offset, so make it is marked as + fully completed. */ + if (inplace) + { + in_uint8s(s, s_remaining(s)); + } + + if (!inplace) + { + s_free(chunk); + } +} + +void +channel_send(STREAM s, VCHANNEL * channel) +{ + uint32 length; + +#ifdef WITH_SCARD + scard_lock(SCARD_LOCK_CHANNEL); +#endif + + s_pop_layer(s, channel_hdr); + in_uint8s(s, 8); + length = s_remaining(s); + + logger(Protocol, Debug, "channel_send(), channel = %d, length = %d", channel->mcs_id, + length); + + while (!s_check_end(s)) + { + channel_send_chunk(s, channel, length); } #ifdef WITH_SCARD diff --git a/stream.h b/stream.h index 45462e0..b9e16dc 100644 --- a/stream.h +++ b/stream.h @@ -138,6 +138,12 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); /* Copy n bytes from array v in to STREAM s */ #define out_uint8a(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } #define out_uint8s(s,n) { s_assert_w(s, n); memset((s)->p,0,n); (s)->p += n; } + +/* Copy n bytes from STREAM s in to STREAM v */ +#define in_uint8stream(s,v,n) { s_assert_r(s, n); out_uint8a((v), (s)->p, n); (s)->p += n; } +/* Copy n bytes in to STREAM s from STREAM v */ +#define out_uint8stream(s,v,n) in_uint8stream(v,s,n) +/* Copy the entire STREAM v (ignoring offsets) in to STREAM s */ #define out_stream(s, v) out_uint8a(s, (v)->data, s_length((v))) #define next_be(s,v) { s_assert_r(s, 1); v = ((v) << 8) + *((s)->p++); } From 489c43f3827a9796972f4052ce3248d0c9191f07 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 13:25:50 +0200 Subject: [PATCH 17/21] Return STREAM objects from data generating functions Use a consistent style of returning a new STREAM object from functions that output data, rather than requiring an existing structure to be passed in. This generally makes the memory management more straight forward and allows us to do more proper bounds checking of everything. This also adds some new STREAM macros to make it easier to manage them without poking around in the internal structure. --- cssp.c | 179 +++++++++++++++++++++++++++++++------------------ proto.h | 2 +- rdpsnd.c | 6 +- rdpsnd.h | 2 +- rdpsnd_alsa.c | 2 +- rdpsnd_dsp.c | 52 +++++++------- rdpsnd_libao.c | 2 +- rdpsnd_oss.c | 2 +- rdpsnd_pulse.c | 2 +- rdpsnd_sgi.c | 2 +- stream.c | 15 +++++ stream.h | 6 ++ tcp.c | 18 +++-- 13 files changed, 179 insertions(+), 111 deletions(-) diff --git a/cssp.c b/cssp.c index 373ed25..c599791 100644 --- a/cssp.c +++ b/cssp.c @@ -139,13 +139,14 @@ cssp_gss_get_service_name(char *server, gss_name_t * name) } -static RD_BOOL -cssp_gss_wrap(gss_ctx_id_t ctx, STREAM in, STREAM out) +static STREAM +cssp_gss_wrap(gss_ctx_id_t ctx, STREAM in) { int conf_state; OM_uint32 major_status; OM_uint32 minor_status; gss_buffer_desc inbuf, outbuf; + STREAM out; inbuf.value = in->data; inbuf.length = s_length(in); @@ -157,35 +158,36 @@ cssp_gss_wrap(gss_ctx_id_t ctx, STREAM in, STREAM out) { cssp_gss_report_error(GSS_C_GSS_CODE, "Failed to encrypt and sign message", major_status, minor_status); - return False; + return NULL; } if (!conf_state) { logger(Core, Error, "cssp_gss_wrap(), GSS Confidentiality failed, no encryption of message performed."); - return False; + return NULL; } // write enc data to out stream - out->data = out->p = xmalloc(outbuf.length); - out->size = outbuf.length; + out = s_alloc(outbuf.length); out_uint8a(out, outbuf.value, outbuf.length); s_mark_end(out); + s_seek(out, 0); gss_release_buffer(&minor_status, &outbuf); - return True; + return out; } -static RD_BOOL -cssp_gss_unwrap(gss_ctx_id_t ctx, STREAM in, STREAM out) +static STREAM +cssp_gss_unwrap(gss_ctx_id_t ctx, STREAM in) { OM_uint32 major_status; OM_uint32 minor_status; gss_qop_t qop_state; gss_buffer_desc inbuf, outbuf; int conf_state; + STREAM out; inbuf.value = in->data; inbuf.length = s_length(in); @@ -196,17 +198,17 @@ cssp_gss_unwrap(gss_ctx_id_t ctx, STREAM in, STREAM out) { cssp_gss_report_error(GSS_C_GSS_CODE, "Failed to decrypt message", major_status, minor_status); - return False; + return NULL; } - out->data = out->p = xmalloc(outbuf.length); - out->size = outbuf.length; + out = s_alloc(outbuf.length); out_uint8a(out, outbuf.value, outbuf.length); s_mark_end(out); + s_seek(out, 0); gss_release_buffer(&minor_status, &outbuf); - return True; + return out; } @@ -585,10 +587,10 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) } -RD_BOOL -cssp_read_tsrequest(STREAM token, STREAM pubkey) +STREAM +cssp_read_tsrequest(RD_BOOL pubkey) { - STREAM s; + STREAM s, out; int length; int tagval; struct stream packet; @@ -596,7 +598,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) s = tcp_recv(NULL, 4); if (s == NULL) - return False; + return NULL; // verify ASN.1 header if (s->p[0] != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) @@ -604,7 +606,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) logger(Protocol, Error, "cssp_read_tsrequest(), expected BER_TAG_SEQUENCE|BER_TAG_CONSTRUCTED, got %x", s->p[0]); - return False; + return NULL; } // peek at first 4 bytes to get full message length @@ -615,7 +617,7 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) else if (s->p[1] == 0x82) length = (s->p[2] << 8) | s->p[3]; else - return False; + return NULL; // receive the remainings of message s = tcp_recv(s, length); @@ -624,12 +626,12 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) // parse the response and into nego token if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) - return False; + return NULL; // version [0] if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0)) - return False; + return NULL; if (!s_check_rem(s, length)) { @@ -639,23 +641,23 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) in_uint8s(s, length); // negoToken [1] - if (token) + if (!pubkey) { if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1)) - return False; + return NULL; if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) - return False; + return NULL; if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) - return False; + return NULL; if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0)) - return False; + return NULL; if (!ber_in_header(s, &tagval, &length) || tagval != BER_TAG_OCTET_STRING) - return False; + return NULL; if (!s_check_rem(s, length)) { @@ -663,29 +665,29 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey) &packet); } - s_realloc(token, length); - s_reset(token); - out_uint8a(token, s->p, length); - s_mark_end(token); + out = s_alloc(length); + out_uint8a(out, s->p, length); + s_mark_end(out); + s_seek(out, 0); } - // pubKey [3] - if (pubkey) + else { if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3)) - return False; + return NULL; if (!ber_in_header(s, &tagval, &length) || tagval != BER_TAG_OCTET_STRING) - return False; + return NULL; - pubkey->data = pubkey->p = s->p; - pubkey->end = pubkey->data + length; - pubkey->size = length; + out = s_alloc(length); + out_uint8a(out, s->p, length); + s_mark_end(out); + s_seek(out, 0); } - return True; + return out; } RD_BOOL @@ -702,9 +704,14 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) gss_OID desired_mech = &_gss_spnego_krb5_mechanism_oid_desc; STREAM ts_creds; - struct stream token = { 0 }; - struct stream pubkey = { 0 }; - struct stream pubkey_cmp = { 0 }; + STREAM token; + STREAM pubkey, pubkey_cmp; + unsigned char *pubkey_data; + unsigned char *pubkey_cmp_data; + unsigned char first_byte; + + RD_BOOL ret; + STREAM blob; // Verify that system gss support spnego if (!cssp_gss_mech_available(desired_mech)) @@ -728,16 +735,19 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) return False; } - tcp_tls_get_server_pubkey(&pubkey); + pubkey = tcp_tls_get_server_pubkey(); + if (pubkey == NULL) + return False; + pubkey_cmp = NULL; // Enter the spnego loop OM_uint32 actual_services; gss_OID actual_mech; - struct stream blob = { 0 }; gss_ctx = GSS_C_NO_CONTEXT; cred = GSS_C_NO_CREDENTIAL; + token = NULL; input_tok.length = 0; output_tok.length = 0; minor_status = 0; @@ -758,6 +768,11 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) &actual_mech, &output_tok, &actual_services, &actual_time); + // input_tok might have pointed to token's data, + // but it's safe to free it now after the call + s_free(token); + token = NULL; + if (GSS_ERROR(major_status)) { if (i == 0) @@ -782,39 +797,44 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) // Send token to server if (output_tok.length != 0) { - if (output_tok.length > token.size) - s_realloc(&token, output_tok.length); - s_reset(&token); + token = s_alloc(output_tok.length); + out_uint8a(token, output_tok.value, output_tok.length); + s_mark_end(token); - out_uint8a(&token, output_tok.value, output_tok.length); - s_mark_end(&token); - - if (!cssp_send_tsrequest(&token, NULL, NULL)) - goto bail_out; + ret = cssp_send_tsrequest(token, NULL, NULL); + s_free(token); + token = NULL; (void) gss_release_buffer(&minor_status, &output_tok); + + if (!ret) + goto bail_out; } // Read token from server if (major_status & GSS_S_CONTINUE_NEEDED) { - (void) gss_release_buffer(&minor_status, &input_tok); - - if (!cssp_read_tsrequest(&token, NULL)) + token = cssp_read_tsrequest(False); + if (token == NULL) goto bail_out; - input_tok.value = token.data; - input_tok.length = s_length(&token); + input_tok.length = s_length(token); + in_uint8p(token, input_tok.value, input_tok.length); } else { // Send encrypted pubkey for verification to server context_established = 1; - if (!cssp_gss_wrap(gss_ctx, &pubkey, &blob)) + blob = cssp_gss_wrap(gss_ctx, pubkey); + if (blob == NULL) goto bail_out; - if (!cssp_send_tsrequest(NULL, NULL, &blob)) + ret = cssp_send_tsrequest(NULL, NULL, blob); + + s_free(blob); + + if (!ret) goto bail_out; context_established = 1; @@ -825,37 +845,62 @@ cssp_connect(char *server, char *user, char *domain, char *password, STREAM s) } while (!context_established); + s_free(token); + // read tsrequest response and decrypt for public key validation - if (!cssp_read_tsrequest(NULL, &blob)) + blob = cssp_read_tsrequest(True); + if (blob == NULL) goto bail_out; - if (!cssp_gss_unwrap(gss_ctx, &blob, &pubkey_cmp)) + pubkey_cmp = cssp_gss_unwrap(gss_ctx, blob); + s_free(blob); + if (pubkey_cmp == NULL) goto bail_out; - pubkey_cmp.data[0] -= 1; + // the first byte gets 1 added before being sent by the server + // in order to protect against replays of the data sent earlier + // by the client + in_uint8(pubkey_cmp, first_byte); + s_seek(pubkey_cmp, 0); + out_uint8(pubkey_cmp, first_byte - 1); + s_seek(pubkey_cmp, 0); // validate public key - if (memcmp(pubkey.data, pubkey_cmp.data, s_length(&pubkey)) != 0) + in_uint8p(pubkey, pubkey_data, s_length(pubkey)); + in_uint8p(pubkey_cmp, pubkey_cmp_data, s_length(pubkey_cmp)); + if ((s_length(pubkey) != s_length(pubkey_cmp)) || + (memcmp(pubkey_data, pubkey_cmp_data, s_length(pubkey)) != 0)) { logger(Core, Error, "cssp_connect(), public key mismatch, cannot guarantee integrity of server connection"); goto bail_out; } + s_free(pubkey); + s_free(pubkey_cmp); + // Send TSCredentials ts_creds = cssp_encode_tscredentials(user, password, domain); - if (!cssp_gss_wrap(gss_ctx, ts_creds, &blob)) - goto bail_out; + blob = cssp_gss_wrap(gss_ctx, ts_creds); s_free(ts_creds); - if (!cssp_send_tsrequest(NULL, &blob, NULL)) + if (blob == NULL) + goto bail_out; + + ret = cssp_send_tsrequest(NULL, blob, NULL); + + s_free(blob); + + if (!ret) goto bail_out; return True; bail_out: - xfree(token.data); + s_free(token); + s_free(pubkey); + s_free(pubkey_cmp); return False; } diff --git a/proto.h b/proto.h index f7f0c6c..2ddb313 100644 --- a/proto.h +++ b/proto.h @@ -227,7 +227,7 @@ char *tcp_get_address(void); RD_BOOL tcp_is_connected(void); void tcp_reset_state(void); RD_BOOL tcp_tls_connect(void); -RD_BOOL tcp_tls_get_server_pubkey(STREAM s); +STREAM tcp_tls_get_server_pubkey(); void tcp_run_ui(RD_BOOL run); /* asn.c */ diff --git a/rdpsnd.c b/rdpsnd.c index b5b8ded..8bb2dbb 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -641,7 +641,7 @@ rdpsnd_queue_write(STREAM s, uint16 tick, uint8 index) queue_hi = next_hi; - packet->s = *s; + packet->s = s; packet->tick = tick; packet->index = index; @@ -675,7 +675,7 @@ rdpsnd_queue_clear(void) while (queue_pending != queue_hi) { packet = &packet_queue[queue_pending]; - xfree(packet->s.data); + s_free(packet->s); queue_pending = (queue_pending + 1) % MAX_QUEUE; } @@ -740,7 +740,7 @@ rdpsnd_queue_complete_pending(void) (packet->completion_tv.tv_usec - packet->arrive_tv.tv_usec); elapsed /= 1000; - xfree(packet->s.data); + s_free(packet->s); rdpsnd_send_waveconfirm((packet->tick + elapsed) % 65536, packet->index); queue_pending = (queue_pending + 1) % MAX_QUEUE; } diff --git a/rdpsnd.h b/rdpsnd.h index 81e87c4..b31e175 100644 --- a/rdpsnd.h +++ b/rdpsnd.h @@ -19,7 +19,7 @@ struct audio_packet { - struct stream s; + STREAM s; uint16 tick; uint8 index; diff --git a/rdpsnd_alsa.c b/rdpsnd_alsa.c index 26e5cc8..25fbc31 100644 --- a/rdpsnd_alsa.c +++ b/rdpsnd_alsa.c @@ -376,7 +376,7 @@ alsa_play(void) return; packet = rdpsnd_queue_current_packet(); - out = &packet->s; + out = packet->s; next_tick = rdpsnd_queue_next_tick(); diff --git a/rdpsnd_dsp.c b/rdpsnd_dsp.c index 9661e09..8cc10fb 100644 --- a/rdpsnd_dsp.c +++ b/rdpsnd_dsp.c @@ -175,8 +175,8 @@ rdpsnd_dsp_resample_supported(RD_WAVEFORMATEX * format) return True; } -uint32 -rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, +STREAM +rdpsnd_dsp_resample(unsigned char *in, unsigned int size, RD_WAVEFORMATEX * format, RD_BOOL stream_be) { UNUSED(stream_be); @@ -190,13 +190,15 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, int innum, outnum; unsigned char *tmpdata = NULL, *tmp = NULL; int samplewidth = format->wBitsPerSample / 8; + STREAM out; int outsize = 0; + unsigned char *data; int i; if ((resample_to_bitspersample == format->wBitsPerSample) && (resample_to_channels == format->nChannels) && (resample_to_srate == format->nSamplesPerSec)) - return 0; + return NULL; #ifdef B_ENDIAN if (!stream_be) @@ -260,7 +262,7 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, { logger(Sound, Warning, "rdpsndp_dsp_resample_set(), no sample rate converter available"); - return 0; + return NULL; } outnum = ((float) innum * ((float) resample_to_srate / (float) format->nSamplesPerSec)) + 1; @@ -285,8 +287,9 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, xfree(infloat); outsize = resample_data.output_frames_gen * resample_to_channels * samplewidth; - *out = (unsigned char *) xmalloc(outsize); - src_float_to_short_array(outfloat, (short *) *out, + out = s_alloc(outsize); + out_uint8p(out, data, outsize); + src_float_to_short_array(outfloat, (short *) data, resample_data.output_frames_gen * resample_to_channels); xfree(outfloat); @@ -302,8 +305,9 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, outnum = (innum * ratio1k) / 1000; outsize = outnum * samplewidth; - *out = (unsigned char *) xmalloc(outsize); - bzero(*out, outsize); + out = s_alloc(outsize); + out_uint8p(out, data, outsize); + bzero(data, outsize); for (i = 0; i < outsize / (resample_to_channels * samplewidth); i++) { @@ -331,7 +335,7 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, cval1 += (sint8) (cval2 * part) / 100; - memcpy(*out + (i * resample_to_channels * samplewidth) + + memcpy(data + (i * resample_to_channels * samplewidth) + (samplewidth * j), &cval1, samplewidth); } } @@ -349,14 +353,14 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, sval1 += (sint16) (sval2 * part) / 100; - memcpy(*out + (i * resample_to_channels * samplewidth) + + memcpy(data + (i * resample_to_channels * samplewidth) + (samplewidth * j), &sval1, samplewidth); } } #else /* Nearest neighbor search */ for (j = 0; j < resample_to_channels; j++) { - memcpy(*out + (i * resample_to_channels * samplewidth) + (samplewidth * j), + memcpy(out + (i * resample_to_channels * samplewidth) + (samplewidth * j), in + (source * resample_to_channels * samplewidth) + (samplewidth * j), samplewidth); } @@ -378,7 +382,7 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, { for (i = 0; i < outsize; i++) { - *out[i] = *out[i * 2]; + data[i] = data[i * 2]; } outsize /= 2; } @@ -386,16 +390,17 @@ rdpsnd_dsp_resample(unsigned char **out, unsigned char *in, unsigned int size, #ifdef B_ENDIAN if (!stream_be) - rdpsnd_dsp_swapbytes(*out, outsize, format); + rdpsnd_dsp_swapbytes(data, outsize, format); #endif - return outsize; + + return out; } STREAM rdpsnd_dsp_process(unsigned char *data, unsigned int size, struct audio_driver * current_driver, RD_WAVEFORMATEX * format) { - static struct stream out; + STREAM out; RD_BOOL stream_be = False; /* softvol and byteswap do not change the amount of data they @@ -411,20 +416,19 @@ rdpsnd_dsp_process(unsigned char *data, unsigned int size, struct audio_driver * } #endif - out.data = NULL; + out = NULL; if (current_driver->need_resampling) - out.size = rdpsnd_dsp_resample(&out.data, data, size, format, stream_be); + out = rdpsnd_dsp_resample(data, size, format, stream_be); - if (out.data == NULL) + if (out == NULL) { - out.data = (unsigned char *) xmalloc(size); - memcpy(out.data, data, size); - out.size = size; + out = s_alloc(size); + out_uint8a(out, data, size); } - out.p = out.data; - out.end = out.p + out.size; + s_mark_end(out); + s_seek(out, 0); - return &out; + return out; } diff --git a/rdpsnd_libao.c b/rdpsnd_libao.c index 6d1996f..3a59788 100644 --- a/rdpsnd_libao.c +++ b/rdpsnd_libao.c @@ -167,7 +167,7 @@ libao_play(void) return; packet = rdpsnd_queue_current_packet(); - out = &packet->s; + out = packet->s; next_tick = rdpsnd_queue_next_tick(); diff --git a/rdpsnd_oss.c b/rdpsnd_oss.c index ae5df21..0705236 100644 --- a/rdpsnd_oss.c +++ b/rdpsnd_oss.c @@ -410,7 +410,7 @@ oss_play(void) return; packet = rdpsnd_queue_current_packet(); - out = &packet->s; + out = packet->s; len = s_remaining(out); diff --git a/rdpsnd_pulse.c b/rdpsnd_pulse.c index 9bcee39..a763541 100644 --- a/rdpsnd_pulse.c +++ b/rdpsnd_pulse.c @@ -1154,7 +1154,7 @@ pulse_play(void) do { packet = rdpsnd_queue_current_packet(); - out = &packet->s; + out = packet->s; ti = pa_stream_get_timing_info(playback_stream); if (ti == NULL) diff --git a/rdpsnd_sgi.c b/rdpsnd_sgi.c index d94f648..d62f12c 100644 --- a/rdpsnd_sgi.c +++ b/rdpsnd_sgi.c @@ -254,7 +254,7 @@ sgi_play(void) return; packet = rdpsnd_queue_current_packet(); - out = (STREAM) (void *) &(packet->s); + out = packet->s; len = s_remaining(out); diff --git a/stream.c b/stream.c index 8400446..a4b9542 100644 --- a/stream.c +++ b/stream.c @@ -37,6 +37,19 @@ s_alloc(unsigned int size) return s; } +STREAM +s_inherit(unsigned char *data, unsigned int size) +{ + STREAM s; + + s = xmalloc(sizeof(struct stream)); + memset(s, 0, sizeof(struct stream)); + s->p = s->data = data; + s->size = size; + + return s; +} + void s_realloc(STREAM s, unsigned int size) { @@ -71,6 +84,8 @@ s_reset(STREAM s) void s_free(STREAM s) { + if (s == NULL) + return; free(s->data); free(s); } diff --git a/stream.h b/stream.h index b9e16dc..5e7464b 100644 --- a/stream.h +++ b/stream.h @@ -43,6 +43,8 @@ typedef struct stream /* Return a newly allocated STREAM object of the specified size */ STREAM s_alloc(unsigned int size); +/* Wrap an existing buffer in a STREAM object, transferring ownership */ +STREAM s_inherit(unsigned char *data, unsigned int size); /* Resize an existing STREAM object, keeping all data and offsets intact */ void s_realloc(STREAM s, unsigned int size); /* Free STREAM object and its associated buffer */ @@ -60,6 +62,10 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define s_push_layer(s,h,n) { (s)->h = (s)->p; (s)->p += n; } #define s_pop_layer(s,h) (s)->p = (s)->h; #define s_mark_end(s) (s)->end = (s)->p; +/* Return current read offset in the STREAM */ +#define s_tell(s) (size_t)((s)->p - (s)->data) +/* Set current read offset in the STREAM */ +#define s_seek(s,o) (s)->p = (s)->data; s_assert_r(s,o); (s)->p += o; /* Returns number of bytes that can still be read from STREAM */ #define s_remaining(s) (size_t)((s)->end - (s)->p) #define s_check_rem(s,n) (((s)->p <= (s)->end) && ((size_t)n <= s_remaining(s))) diff --git a/tcp.c b/tcp.c index 726f6c0..8634631 100644 --- a/tcp.c +++ b/tcp.c @@ -399,8 +399,8 @@ fail: } /* Get public key from server of TLS 1.x connection */ -RD_BOOL -tcp_tls_get_server_pubkey(STREAM s) +STREAM +tcp_tls_get_server_pubkey() { int ret; unsigned int list_size; @@ -413,8 +413,7 @@ tcp_tls_get_server_pubkey(STREAM s) int pk_size; uint8_t pk_data[1024]; - s->data = s->p = NULL; - s->size = 0; + STREAM s = NULL; cert_list = gnutls_certificate_get_peers(g_tls_session, &list_size); @@ -466,11 +465,10 @@ tcp_tls_get_server_pubkey(STREAM s) goto out; } - s->size = pk_size; - s->data = s->p = xmalloc(s->size); - memcpy((void *)s->data, (void *)pk_data, pk_size); - s->p = s->data; - s->end = s->p + s->size; + s = s_alloc(pk_size); + out_uint8a(s, pk_data, pk_size); + s_mark_end(s); + s_seek(s, 0); out: if ((e.size != 0) && (e.data)) { @@ -481,7 +479,7 @@ out: free(m.data); } - return (s->size != 0); + return s; } /* Helper function to determine if rdesktop should resolve hostnames again or not */ From 25b84123336bec6e85cb0b7d61c6fb902ae44825 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 9 Apr 2019 13:35:46 +0200 Subject: [PATCH 18/21] Avoid poking around in STREAM internals It's easy to make mistakes this way, and bypassed the normal bounds checking. So make sure we always use macros or functions. --- channels.c | 17 ++---- cliprdr.c | 9 +-- cssp.c | 87 ++++++++++++-------------- licence.c | 10 ++- lspci.c | 3 +- mcs.c | 9 +-- orders.c | 12 ++-- printercache.c | 16 +++-- rdp.c | 73 +++++++++++++--------- rdp5.c | 30 ++++----- rdpdr.c | 119 +++++++++++++++++++----------------- rdpsnd.c | 58 +++++++++--------- rdpsnd_alsa.c | 16 ++++- rdpsnd_libao.c | 7 ++- rdpsnd_oss.c | 14 +++-- rdpsnd_pulse.c | 11 ++-- rdpsnd_sgi.c | 7 ++- rdpsnd_sun.c | 16 +++-- scard.c | 162 +++++++++++++++++++++++++++++-------------------- seamless.c | 3 +- secure.c | 91 +++++++++++++++++---------- stream.h | 3 + tcp.c | 55 +++++++++-------- 23 files changed, 469 insertions(+), 359 deletions(-) diff --git a/channels.c b/channels.c index b5ea690..e51f1cc 100644 --- a/channels.c +++ b/channels.c @@ -186,7 +186,6 @@ void channel_process(STREAM s, uint16 mcs_channel) { uint32 length, flags; - uint32 thislength; VCHANNEL *channel = NULL; unsigned int i; STREAM in; @@ -214,22 +213,16 @@ channel_process(STREAM s, uint16 mcs_channel) in = &channel->in; if (flags & CHANNEL_FLAG_FIRST) { - if (length > in->size) - { - in->data = (uint8 *) xrealloc(in->data, length); - in->size = length; - } - in->p = in->data; + s_realloc(in, length); + s_reset(in); } - thislength = MIN(s_remaining(s), in->data + in->size - in->p); - memcpy(in->p, s->p, thislength); - in->p += thislength; + out_uint8stream(in, s, s_remaining(s)); if (flags & CHANNEL_FLAG_LAST) { - in->end = in->p; - in->p = in->data; + s_mark_end(in); + s_seek(in, 0); channel->process(in); } } diff --git a/cliprdr.c b/cliprdr.c index c05e281..a7d41b2 100644 --- a/cliprdr.c +++ b/cliprdr.c @@ -119,21 +119,14 @@ cliprdr_process(STREAM s) uint16 type, status; uint32 length, format; uint8 *data; - struct stream packet = *s; in_uint16_le(s, type); in_uint16_le(s, status); in_uint32_le(s, length); - data = s->p; logger(Clipboard, Debug, "cliprdr_process(), type=%d, status=%d, length=%d", type, status, length); - if (!s_check_rem(s, length)) - { - rdp_protocol_error("consume of packet from stream would overrun", &packet); - } - if (status == CLIPRDR_ERROR) { switch (type) @@ -161,6 +154,7 @@ cliprdr_process(STREAM s) ui_clip_sync(); break; case CLIPRDR_FORMAT_ANNOUNCE: + in_uint8p(s, data, length); ui_clip_format_announce(data, length); cliprdr_send_packet(CLIPRDR_FORMAT_ACK, CLIPRDR_RESPONSE, NULL, 0); return; @@ -171,6 +165,7 @@ cliprdr_process(STREAM s) ui_clip_request_data(format); break; case CLIPRDR_DATA_RESPONSE: + in_uint8p(s, data, length); ui_clip_handle_data(data, length); break; case 7: /* TODO: W2K3 SP1 sends this on connect with a value of 1 */ diff --git a/cssp.c b/cssp.c index c599791..c7b015c 100644 --- a/cssp.c +++ b/cssp.c @@ -38,7 +38,7 @@ ber_wrap_hdr_data(int tagval, STREAM in) out = s_alloc(size); ber_out_header(out, tagval, s_length(in)); - out_uint8a(out, in->data, s_length(in)); + out_stream(out, in); s_mark_end(out); return out; @@ -148,8 +148,10 @@ cssp_gss_wrap(gss_ctx_id_t ctx, STREAM in) gss_buffer_desc inbuf, outbuf; STREAM out; - inbuf.value = in->data; + s_seek(in, 0); inbuf.length = s_length(in); + in_uint8p(in, inbuf.value, s_length(in)); + s_seek(in, 0); major_status = gss_wrap(&minor_status, ctx, True, GSS_C_QOP_DEFAULT, &inbuf, &conf_state, &outbuf); @@ -189,8 +191,10 @@ cssp_gss_unwrap(gss_ctx_id_t ctx, STREAM in) int conf_state; STREAM out; - inbuf.value = in->data; + s_seek(in, 0); inbuf.length = s_length(in); + in_uint8p(in, inbuf.value, s_length(in)); + s_seek(in, 0); major_status = gss_unwrap(&minor_status, ctx, &inbuf, &outbuf, &conf_state, &qop_state); @@ -231,7 +235,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -244,7 +248,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -256,7 +260,7 @@ cssp_encode_tspasswordcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -292,7 +296,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -306,7 +310,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -321,7 +325,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -336,7 +340,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -351,7 +355,7 @@ cssp_encode_tscspdatadetail(unsigned char keyspec, char *card, char *reader, cha h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 4, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -384,7 +388,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -394,7 +398,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) g_sc_container_name, g_sc_csp_name); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -408,7 +412,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -423,7 +427,7 @@ cssp_encode_tssmartcardcreds(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -464,7 +468,7 @@ cssp_encode_tscredentials(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -482,7 +486,7 @@ cssp_encode_tscredentials(char *username, char *password, char *domain) h2 = ber_wrap_hdr_data(BER_TAG_OCTET_STRING, h3); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h3); s_free(h2); @@ -518,7 +522,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h2 = ber_wrap_hdr_data(BER_TAG_INTEGER, &tmp); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -532,7 +536,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h2 = ber_wrap_hdr_data(BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED, h3); h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 1, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h5); s_free(h4); @@ -548,7 +552,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 2, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_free(h2); s_free(h1); @@ -561,7 +565,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) h1 = ber_wrap_hdr_data(BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 3, h2); s_realloc(&message, s_length(&message) + s_length(h1)); - out_uint8a(&message, h1->data, s_length(h1)); + out_stream(&message, h1); s_mark_end(&message); s_free(h2); s_free(h1); @@ -572,7 +576,7 @@ cssp_send_tsrequest(STREAM token, STREAM auth, STREAM pubkey) // Todo: can h1 be send directly instead of tcp_init() approach h1 = ber_wrap_hdr_data(BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED, &message); s = tcp_init(s_length(h1)); - out_uint8a(s, h1->data, s_length(h1)); + out_stream(s, h1); s_mark_end(s); s_free(h1); @@ -600,34 +604,21 @@ cssp_read_tsrequest(RD_BOOL pubkey) if (s == NULL) return NULL; - // verify ASN.1 header - if (s->p[0] != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) - { - logger(Protocol, Error, - "cssp_read_tsrequest(), expected BER_TAG_SEQUENCE|BER_TAG_CONSTRUCTED, got %x", - s->p[0]); - return NULL; - } - - // peek at first 4 bytes to get full message length - if (s->p[1] < 0x80) - length = s->p[1] - 2; - else if (s->p[1] == 0x81) - length = s->p[2] - 1; - else if (s->p[1] == 0x82) - length = (s->p[2] << 8) | s->p[3]; - else - return NULL; - - // receive the remainings of message - s = tcp_recv(s, length); - packet = *s; - - // parse the response and into nego token + // get and verify the header if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED)) return NULL; + // We've already read 4 bytes, but the header might have been + // less than that, so we need to adjust the length + length -= s_remaining(s); + + // receive the remainings of message + s = tcp_recv(s, length); + if (s == NULL) + return NULL; + packet = *s; + // version [0] if (!ber_in_header(s, &tagval, &length) || tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0)) @@ -666,7 +657,7 @@ cssp_read_tsrequest(RD_BOOL pubkey) } out = s_alloc(length); - out_uint8a(out, s->p, length); + out_uint8stream(out, s, length); s_mark_end(out); s_seek(out, 0); } @@ -681,7 +672,7 @@ cssp_read_tsrequest(RD_BOOL pubkey) return NULL; out = s_alloc(length); - out_uint8a(out, s->p, length); + out_uint8stream(out, s, length); s_mark_end(out); s_seek(out, 0); } diff --git a/licence.c b/licence.c index 2125254..4acac8d 100644 --- a/licence.c +++ b/licence.c @@ -277,6 +277,7 @@ licence_process_platform_challenge(STREAM s) static void licence_process_new_license(STREAM s) { + unsigned char *data; RDSSL_RC4 crypt_key; uint32 length; int i; @@ -286,8 +287,12 @@ licence_process_new_license(STREAM s) if (!s_check_rem(s, length)) return; + inout_uint8p(s, data, length); + rdssl_rc4_set_key(&crypt_key, g_licence_key, 16); - rdssl_rc4_crypt(&crypt_key, s->p, s->p, length); + rdssl_rc4_crypt(&crypt_key, data, data, length); + + s_seek(s, s_tell(s) - length); /* Parse NEW_LICENSE_INFO block */ in_uint8s(s, 4); // skip dwVersion @@ -304,7 +309,8 @@ licence_process_new_license(STREAM s) } g_licence_issued = True; - save_licence(s->p, length); + in_uint8p(s, data, length); + save_licence(data, length); } /* process a licence error alert packet */ diff --git a/lspci.c b/lspci.c index 73b974d..1a79131 100644 --- a/lspci.c +++ b/lspci.c @@ -139,7 +139,8 @@ lspci_process(STREAM s) pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = xmalloc(pkglen + 1); - STRNCPY(buf, (char *) s->p, pkglen + 1); + in_uint8a(s, buf, pkglen); + buf[pkglen] = '\0'; str_handle_lines(buf, &rest, lspci_process_line, NULL); xfree(buf); } diff --git a/mcs.c b/mcs.c index a13092b..62d18d8 100644 --- a/mcs.c +++ b/mcs.c @@ -62,7 +62,7 @@ mcs_parse_domain_params(STREAM s) static void mcs_send_connect_initial(STREAM mcs_data) { - int datalen = mcs_data->end - mcs_data->data; + int datalen = s_length(mcs_data); int length = 9 + 3 * 34 + 4 + datalen; STREAM s; logger(Protocol, Debug, "%s()", __func__); @@ -139,9 +139,10 @@ mcs_recv_connect_response(STREAM mcs_data) length = mcs_data->size; } - in_uint8a(s, mcs_data->data, length); - mcs_data->p = mcs_data->data; - mcs_data->end = mcs_data->data + length; + s_reset(mcs_data); + in_uint8stream(s, mcs_data, length); + s_mark_end(mcs_data); + s_seek(mcs_data, 0); */ return s_check_end(s); } diff --git a/orders.c b/orders.c index 44e5034..e463fed 100644 --- a/orders.c +++ b/orders.c @@ -20,7 +20,7 @@ #include "rdesktop.h" #include "orders.h" -extern uint8 *g_next_packet; +extern size_t g_next_packet; static RDP_ORDER_STATE g_order_state; extern RDP_VERSION g_rdp_version; @@ -1252,7 +1252,7 @@ process_secondary_order(STREAM s) sint16 length; uint16 flags; uint8 type; - uint8 *next_order; + size_t next_order; struct stream packet = *s; in_uint16_le(s, length); @@ -1269,7 +1269,7 @@ process_secondary_order(STREAM s) rdp_protocol_error("next order pointer would overrun stream", &packet); } - next_order = s->p + length; + next_order = s_tell(s) + length; switch (type) { @@ -1306,7 +1306,7 @@ process_secondary_order(STREAM s) "process_secondary_order(), unhandled secondary order %d", type); } - s->p = next_order; + s_seek(s, next_order); } /* Process an order PDU */ @@ -1448,9 +1448,9 @@ process_orders(STREAM s, uint16 num_orders) } #if 0 /* not true when RDP_COMPRESSION is set */ - if (s->p != g_next_packet) + if (s_tell(s) != g_next_packet) logger(Graphics, Error, "process_orders(), %d bytes remaining", - (int) (g_next_packet - s->p)); + (int) (g_next_packet - s_tell(s))); #endif } diff --git a/printercache.c b/printercache.c index e0a449c..86973bc 100644 --- a/printercache.c +++ b/printercache.c @@ -238,6 +238,8 @@ printercache_process(STREAM s) { uint32 type, printer_length, driver_length, printer_unicode_length, blob_length; char device_name[9], *printer, *driver; + size_t blob_start; + unsigned char *blob; printer = driver = NULL; @@ -279,8 +281,10 @@ printercache_process(STREAM s) { rdp_in_unistr(s, printer_unicode_length, &printer, &printer_unicode_length); - if (printer) - printercache_save_blob(printer, s->p, blob_length); + if (printer) { + in_uint8p(s, blob, blob_length); + printercache_save_blob(printer, blob, blob_length); + } free(printer); } break; @@ -289,6 +293,7 @@ printercache_process(STREAM s) in_uint8a(s, device_name, 5); /* get LPTx/COMx name */ /* need to fetch this data so that we can get the length of the packet to store. */ + blob_start = s_tell(s); in_uint8s(s, 0x2); /* ??? */ in_uint8s(s, 0x2) /* pad?? */ in_uint32_be(s, driver_length); @@ -302,10 +307,11 @@ printercache_process(STREAM s) /* rewind stream so that we can save this blob */ /* length is driver_length + printer_length + 19 */ /* rewind stream */ - s->p = s->p - 19; + s_seek(s, blob_start); - printercache_save_blob(device_name, s->p, - driver_length + printer_length + 19); + blob_length = driver_length + printer_length + 19; + in_uint8p(s, blob, blob_length); + printercache_save_blob(device_name, blob, blob_length); break; default: logger(Protocol, Warning, diff --git a/rdp.c b/rdp.c index ffc8920..b938c2f 100644 --- a/rdp.c +++ b/rdp.c @@ -59,7 +59,7 @@ extern RD_BOOL g_dynamic_session_resize; RD_BOOL g_exit_mainloop = False; -uint8 *g_next_packet; +size_t g_next_packet; uint32 g_rdp_shareid; extern RDPCOMP g_mppc_dict; @@ -128,6 +128,9 @@ rdp_ts_in_share_control_header(STREAM s, uint8 * type, uint16 * length) *type = pdu_type & 0xf; + /* Give just the size of the data */ + *length -= 6; + return True; } @@ -142,7 +145,7 @@ rdp_recv(uint8 * type) while (1) { /* fill stream with data if needed for parsing a new packet */ - if ((rdp_s == NULL) || (g_next_packet >= rdp_s->end) || (g_next_packet == NULL)) + if (g_next_packet == 0) { rdp_s = sec_recv(&is_fastpath); if (rdp_s == NULL) @@ -155,11 +158,16 @@ rdp_recv(uint8 * type) continue; } - g_next_packet = rdp_s->p; + g_next_packet = s_tell(rdp_s); } else { - rdp_s->p = g_next_packet; + s_seek(rdp_s, g_next_packet); + if (s_check_end(rdp_s)) + { + g_next_packet = 0; + continue; + } } /* parse a TS_SHARECONTROLHEADER */ @@ -171,7 +179,13 @@ rdp_recv(uint8 * type) logger(Protocol, Debug, "rdp_recv(), RDP packet #%d, type 0x%x", ++g_packetno, *type); - g_next_packet += length; + if (!s_check_rem(rdp_s, length)) + { + rdp_protocol_error("not enough data for PDU", rdp_s); + } + + g_next_packet = s_tell(rdp_s) + length; + return rdp_s; } @@ -241,7 +255,8 @@ rdp_out_unistr(STREAM s, char *string, int len) */ static iconv_t icv_local_to_utf16; size_t ibl, obl; - char *pin, *pout; + char *pin; + unsigned char *pout; if (string == NULL || len == 0) @@ -263,18 +278,16 @@ rdp_out_unistr(STREAM s, char *string, int len) ibl = strlen(string); obl = len + 2; pin = string; - pout = (char *) s->p; + out_uint8p(s, pout, len + 2); - memset(pout, 0, len + 4); + memset(pout, 0, len + 2); - if (iconv(icv_local_to_utf16, (char **) &pin, &ibl, &pout, &obl) == (size_t) - 1) + if (iconv(icv_local_to_utf16, (char **) &pin, &ibl, (char **)&pout, &obl) == (size_t) - 1) { logger(Protocol, Error, "rdp_out_unistr(), iconv(2) fail, errno %d", errno); abort(); } - - s->p += len + 2; } /* Input a string in Unicode @@ -286,7 +299,8 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) { static iconv_t icv_utf16_to_local; size_t ibl, obl; - char *pin, *pout; + unsigned char *pin; + char *pout; struct stream packet = *s; @@ -323,7 +337,7 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) ibl = in_len; obl = *str_size - 1; - pin = (char *) s->p; + in_uint8p(s, pin, in_len); pout = *string; if (iconv(icv_utf16_to_local, (char **) &pin, &ibl, &pout, &obl) == (size_t) - 1) @@ -344,9 +358,6 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) abort(); } - /* we must update the location of the current STREAM for future reads of s->p */ - s->p += in_len; - *pout = 0; if (*string) @@ -1219,25 +1230,25 @@ static void rdp_process_server_caps(STREAM s, uint16 length) { int n; - uint8 *next, *start; + size_t next, start; uint16 ncapsets, capset_type, capset_length; logger(Protocol, Debug, "%s()", __func__); - start = s->p; + start = s_tell(s); in_uint16_le(s, ncapsets); in_uint8s(s, 2); /* pad */ for (n = 0; n < ncapsets; n++) { - if (s->p > start + length) + if (s_tell(s) > start + length) return; in_uint16_le(s, capset_type); in_uint16_le(s, capset_length); - next = s->p + capset_length - 4; + next = s_tell(s) + capset_length - 4; switch (capset_type) { @@ -1256,7 +1267,7 @@ rdp_process_server_caps(STREAM s, uint16 length) break; } - s->p = next; + s_seek(s, next); } } @@ -1726,6 +1737,7 @@ process_data_pdu(STREAM s, uint32 * ext_disc_reason) uint16 clen; uint32 len; + uint8 *buf; uint32 roff, rlen; struct stream *ns = &(g_mppc_dict.ns); @@ -1742,21 +1754,22 @@ process_data_pdu(STREAM s, uint32 * ext_disc_reason) if (len > RDP_MPPC_DICT_SIZE) logger(Protocol, Error, "process_data_pdu(), error decompressed packet size exceeds max"); - if (mppc_expand(s->p, clen, ctype, &roff, &rlen) == -1) + in_uint8p(s, buf, clen); + if (mppc_expand(buf, clen, ctype, &roff, &rlen) == -1) logger(Protocol, Error, "process_data_pdu(), error while decompressing packet"); /* len -= 18; */ /* allocate memory and copy the uncompressed data into the temporary stream */ - ns->data = (uint8 *) xrealloc(ns->data, rlen); + s_realloc(ns, rlen); + s_reset(ns); - memcpy((ns->data), (unsigned char *) (g_mppc_dict.hist + roff), rlen); + out_uint8a(ns, (unsigned char *) (g_mppc_dict.hist + roff), rlen); - ns->size = rlen; - ns->end = (ns->data + ns->size); - ns->p = ns->data; - ns->rdp_hdr = ns->p; + s_mark_end(ns); + s_seek(ns, 0); + s_push_layer(ns, rdp_hdr, 0); s = ns; } @@ -2056,7 +2069,7 @@ rdp_loop(RD_BOOL * deactivated, uint32 * ext_disc_reason) logger(Protocol, Warning, "rdp_loop(), unhandled PDU type %d received", type); } - cont = g_next_packet < s->end; + cont = g_next_packet < s_length(s); } return True; } @@ -2094,7 +2107,7 @@ void rdp_reset_state(void) { logger(Protocol, Debug, "%s()", __func__); - g_next_packet = NULL; /* reset the packet information */ + g_next_packet = 0; /* reset the packet information */ g_rdp_shareid = 0; g_exit_mainloop = False; g_first_bitmap_caps = True; diff --git a/rdp5.c b/rdp5.c index 5a8de96..6b7ab05 100644 --- a/rdp5.c +++ b/rdp5.c @@ -21,7 +21,7 @@ #include "rdesktop.h" -extern uint8 *g_next_packet; +extern size_t g_next_packet; extern RDPCOMP g_mppc_dict; @@ -78,8 +78,9 @@ process_ts_fp_updates(STREAM s) { uint16 length; uint8 hdr, code, frag, comp, ctype = 0; - uint8 *next; + size_t next; + uint8 *buf; uint32 roff, rlen; struct stream *ns = &(g_mppc_dict.ns); struct stream *ts; @@ -87,7 +88,7 @@ process_ts_fp_updates(STREAM s) static STREAM assembled[16] = { 0 }; ui_begin_update(); - while (s->p < s->end) + while (!s_check_end(s)) { /* Reading a number of TS_FP_UPDATE structures from the stream here.. */ in_uint8(s, hdr); /* updateHeader */ @@ -100,23 +101,24 @@ process_ts_fp_updates(STREAM s) in_uint16_le(s, length); /* length */ - g_next_packet = next = s->p + length; + g_next_packet = next = s_tell(s) + length; if (ctype & RDP_MPPC_COMPRESSED) { - if (mppc_expand(s->p, length, ctype, &roff, &rlen) == -1) + in_uint8p(s, buf, length); + if (mppc_expand(buf, length, ctype, &roff, &rlen) == -1) logger(Protocol, Error, "process_ts_fp_update_pdu(), error while decompressing packet"); /* allocate memory and copy the uncompressed data into the temporary stream */ - ns->data = (uint8 *) xrealloc(ns->data, rlen); + s_realloc(ns, rlen); + s_reset(ns); - memcpy((ns->data), (unsigned char *) (g_mppc_dict.hist + roff), rlen); + out_uint8a(ns, (unsigned char *) (g_mppc_dict.hist + roff), rlen); - ns->size = rlen; - ns->end = (ns->data + ns->size); - ns->p = ns->data; - ns->rdp_hdr = ns->p; + s_mark_end(ns); + s_seek(ns, 0); + s_push_layer(ns, rdp_hdr, 0); length = rlen; ts = ns; @@ -140,17 +142,17 @@ process_ts_fp_updates(STREAM s) s_reset(assembled[code]); } - out_uint8a(assembled[code], ts->p, length); + out_uint8stream(assembled[code], ts, length); if (frag == FASTPATH_FRAGMENT_LAST) { s_mark_end(assembled[code]); - assembled[code]->p = assembled[code]->data; + s_seek(assembled[code], 0); process_ts_fp_update_by_code(assembled[code], code); } } - s->p = next; + s_seek(s, next); } ui_end_update(); } diff --git a/rdpdr.c b/rdpdr.c index 0221366..333f6d1 100644 --- a/rdpdr.c +++ b/rdpdr.c @@ -411,7 +411,6 @@ rdpdr_process_irp(STREAM s) request, file, info_level, - buffer_len, id, major, minor, @@ -424,8 +423,8 @@ rdpdr_process_irp(STREAM s) char *filename; uint32 filename_len; - uint8 *buffer, *pst_buf; - struct stream out; + uint8 *pst_buf; + STREAM out; DEVICE_FNS *fns; RD_BOOL rw_blocking = True; RD_NTSTATUS status = RD_STATUS_INVALID_DEVICE_REQUEST; @@ -438,16 +437,13 @@ rdpdr_process_irp(STREAM s) filename = NULL; - buffer_len = 0; - buffer = (uint8 *) xmalloc(1024); - buffer[0] = 0; + out = NULL; if (device >= RDPDR_MAX_DEVICES) { logger(Protocol, Error, "rdpdr_process_irp(), invalid irp device=0x%lx, file=0x%lx, id=0x%lx, major=0x%lx, minor=0x%lx", device, file, id, major, minor); - xfree(buffer); return; } @@ -486,7 +482,6 @@ rdpdr_process_irp(STREAM s) logger(Protocol, Error, "rdpdr_process_irp(), received IRP for unknown device type %ld", device); - xfree(buffer); return; } @@ -520,7 +515,9 @@ rdpdr_process_irp(STREAM s) flags_and_attributes, filename, &result); free(filename); - buffer_len = 1; + out = s_alloc(1); + out_uint8(out, 0); + s_mark_end(out); break; case IRP_MJ_CLOSE: @@ -557,14 +554,14 @@ rdpdr_process_irp(STREAM s) if (rw_blocking) /* Complete read immediately */ { - buffer = (uint8 *) xrealloc((void *) buffer, length); - if (!buffer) - { - status = RD_STATUS_CANCELLED; - break; - } + uint8* buffer; + out = s_alloc(length); + out_uint8p(out, buffer, length); status = fns->read(file, buffer, length, offset, &result); - buffer_len = result; + /* Might have read less */ + s_mark_end(out); + s_seek(out, result); + s_mark_end(out); break; } @@ -588,7 +585,9 @@ rdpdr_process_irp(STREAM s) break; case IRP_MJ_WRITE: - buffer_len = 1; + out = s_alloc(1); + out_uint8(out, 0); + s_mark_end(out); if (!fns->write) { @@ -612,7 +611,9 @@ rdpdr_process_irp(STREAM s) if (rw_blocking) /* Complete immediately */ { - status = fns->write(file, s->p, length, offset, &result); + unsigned char *data; + in_uint8p(s, data, length); + status = fns->write(file, data, length, offset, &result); break; } @@ -645,10 +646,10 @@ rdpdr_process_irp(STREAM s) } in_uint32_le(s, info_level); - out.data = out.p = buffer; - out.size = sizeof(buffer); - status = disk_query_information(file, info_level, &out); - result = buffer_len = out.p - out.data; + out = s_alloc(1024); + status = disk_query_information(file, info_level, out); + s_mark_end(out); + result = s_length(out); break; @@ -662,10 +663,10 @@ rdpdr_process_irp(STREAM s) in_uint32_le(s, info_level); - out.data = out.p = buffer; - out.size = sizeof(buffer); - status = disk_set_information(file, info_level, s, &out); - result = buffer_len = out.p - out.data; + out = s_alloc(1024); + status = disk_set_information(file, info_level, s, out); + s_mark_end(out); + result = s_length(out); break; case IRP_MJ_QUERY_VOLUME_INFORMATION: @@ -678,10 +679,10 @@ rdpdr_process_irp(STREAM s) in_uint32_le(s, info_level); - out.data = out.p = buffer; - out.size = sizeof(buffer); - status = disk_query_volume_information(file, info_level, &out); - result = buffer_len = out.p - out.data; + out = s_alloc(1024); + status = disk_query_volume_information(file, info_level, out); + s_mark_end(out); + result = s_length(out); break; case IRP_MJ_DIRECTORY_CONTROL: @@ -707,13 +708,16 @@ rdpdr_process_irp(STREAM s) convert_to_unix_filename(filename); } - out.data = out.p = buffer; - out.size = sizeof(buffer); + out = s_alloc(1024); status = disk_query_directory(file, info_level, filename, - &out); - result = buffer_len = out.p - out.data; - if (!buffer_len) - buffer_len++; + out); + s_mark_end(out); + if (!s_length(out)) + { + out_uint8(out, 0); + s_mark_end(out); + } + result = s_length(out); free(filename); break; @@ -760,23 +764,14 @@ rdpdr_process_irp(STREAM s) in_uint8s(s, 0x14); /* TODO: Why do we need to increase length by padlen? Or is it hdr len? */ - buffer = (uint8 *) xrealloc((void *) buffer, bytes_out + 0x14); - if (!buffer) - { - status = RD_STATUS_CANCELLED; - break; - } - - out.data = out.p = buffer; - /* Guess, just a simple mistype. Check others */ - //out.size = sizeof(buffer); - out.size = bytes_out + 0x14; + out = s_alloc(bytes_out + 0x14); #ifdef WITH_SCARD scardSetInfo(g_epoch, device, id, bytes_out + 0x14); #endif - status = fns->device_control(file, request, s, &out); - result = buffer_len = out.p - out.data; + status = fns->device_control(file, request, s, out); + s_mark_end(out); + result = s_length(out); /* Serial SERIAL_WAIT_ON_MASK */ if (status == RD_STATUS_PENDING) @@ -805,12 +800,12 @@ rdpdr_process_irp(STREAM s) in_uint32_le(s, info_level); - out.data = out.p = buffer; - out.size = sizeof(buffer); + out = s_alloc(1024); /* FIXME: Perhaps consider actually *do* something here :-) */ status = RD_STATUS_SUCCESS; - result = buffer_len = out.p - out.data; + s_mark_end(out); + result = s_length(out); break; default: @@ -822,11 +817,25 @@ rdpdr_process_irp(STREAM s) if (status != RD_STATUS_PENDING) { + size_t buffer_len; + uint8 *buffer; + + if (out != NULL) + { + buffer_len = s_length(out); + s_seek(out, 0); + in_uint8p(out, buffer, buffer_len); + } + else + { + buffer_len = 0; + buffer = NULL; + } + rdpdr_send_completion(device, id, status, result, buffer, buffer_len); } - if (buffer) - xfree(buffer); - buffer = NULL; + if (out) + s_free(out); } static void diff --git a/rdpsnd.c b/rdpsnd.c index 8bb2dbb..7bbb9b6 100644 --- a/rdpsnd.c +++ b/rdpsnd.c @@ -64,6 +64,7 @@ unsigned int queue_hi, queue_lo, queue_pending; struct audio_packet packet_queue[MAX_QUEUE]; static uint8 packet_opcode; +static size_t packet_len; static struct stream packet; void (*wave_out_play) (void); @@ -295,8 +296,11 @@ static void rdpsnd_process_packet(uint8 opcode, STREAM s) { uint16 vol_left, vol_right; - static uint16 tick, format; - static uint8 packet_index; + + uint16 tick, format; + uint8 packet_index; + unsigned int size; + unsigned char *data; switch (opcode) { @@ -345,9 +349,12 @@ rdpsnd_process_packet(uint8 opcode, STREAM s) current_format = format; } - rdpsnd_queue_write(rdpsnd_dsp_process - (s->p, s_remaining(s), current_driver, - &formats[current_format]), tick, packet_index); + size = s_remaining(s); + in_uint8p(s, data, size); + rdpsnd_queue_write(rdpsnd_dsp_process(data, size, + current_driver, + &formats[current_format]), + tick, packet_index); return; break; case SNDC_CLOSE: @@ -382,12 +389,10 @@ rdpsnd_process_packet(uint8 opcode, STREAM s) static void rdpsnd_process(STREAM s) { - uint16 len; - while (!s_check_end(s)) { /* New packet */ - if (packet.size == 0) + if (packet_len == 0) { if (!s_check_rem(s, 4)) { @@ -397,25 +402,23 @@ rdpsnd_process(STREAM s) } in_uint8(s, packet_opcode); in_uint8s(s, 1); /* Padding */ - in_uint16_le(s, len); + in_uint16_le(s, packet_len); logger(Sound, Debug, "rdpsnd_process(), Opcode = 0x%x Length= %d", - (int) packet_opcode, (int) len); - - packet.p = packet.data; - packet.end = packet.data + len; - packet.size = len; + (int) packet_opcode, (int) packet_len); } else { - len = MIN(s_remaining(s), s_remaining(&packet)); + uint16 len; + + len = MIN(s_remaining(s), packet_len - s_length(&packet)); /* Microsoft's server is so broken it's not even funny... */ if (packet_opcode == SNDC_WAVE) { - if ((packet.p - packet.data) < 12) - len = MIN(len, 12 - (packet.p - packet.data)); - else if ((packet.p - packet.data) == 12) + if (s_length(&packet) < 12) + len = MIN(len, 12 - s_length(&packet)); + else if (s_length(&packet) == 12) { logger(Sound, Debug, "rdpsnd_process(), eating 4 bytes of %d bytes...", @@ -425,16 +428,18 @@ rdpsnd_process(STREAM s) } } - in_uint8a(s, packet.p, len); - packet.p += len; + in_uint8stream(s, &packet, len); + /* Always end it so s_length() works */ + s_mark_end(&packet); } /* Packet fully assembled */ - if (packet.p == packet.end) + if (s_length(&packet) == packet_len) { - packet.p = packet.data; + s_seek(&packet, 0); rdpsnd_process_packet(packet_opcode, &packet); - packet.size = 0; + packet_len = 0; + s_reset(&packet); } } } @@ -457,7 +462,8 @@ rdpsnddbg_process(STREAM s) pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = (char *) xmalloc(pkglen + 1); - STRNCPY(buf, (char *) s->p, pkglen + 1); + in_uint8a(s, buf, pkglen); + buf[pkglen] = '\0'; str_handle_lines(buf, &rest, rdpsnddbg_line_handler, NULL); @@ -513,9 +519,7 @@ rdpsnd_init(char *optarg) drivers = NULL; - packet.data = (uint8 *) xmalloc(65536); - packet.p = packet.end = packet.data; - packet.size = 0; + s_realloc(&packet, 65536); rdpsnd_channel = channel_register("rdpsnd", CHANNEL_OPTION_INITIALIZED | CHANNEL_OPTION_ENCRYPT_RDP, diff --git a/rdpsnd_alsa.c b/rdpsnd_alsa.c index 25fbc31..0a71597 100644 --- a/rdpsnd_alsa.c +++ b/rdpsnd_alsa.c @@ -358,6 +358,8 @@ alsa_play(void) struct audio_packet *packet; STREAM out; int len; + const unsigned char *data; + size_t before; static long prev_s, prev_us; int duration; struct timeval tv; @@ -380,13 +382,21 @@ alsa_play(void) next_tick = rdpsnd_queue_next_tick(); + before = s_tell(out); + len = s_remaining(out) / (samplewidth_out * audiochannels_out); - if ((len = snd_pcm_writei(out_handle, out->p, ((MAX_FRAMES < len) ? MAX_FRAMES : len))) < 0) + len = MIN(len, MAX_FRAMES); + in_uint8p(out, data, len); + + len = snd_pcm_writei(out_handle, data, len); + if (len < 0) { snd_pcm_prepare(out_handle); len = 0; } - out->p += (len * samplewidth_out * audiochannels_out); + + /* We might not have written everything */ + s_seek(out, before + len * samplewidth_out * audiochannels_out); gettimeofday(&tv, NULL); @@ -395,7 +405,7 @@ alsa_play(void) if (packet->tick > next_tick) next_tick += 65536; - if ((out->p == out->end) || duration > next_tick - packet->tick + 500) + if (s_check_end(out) || duration > next_tick - packet->tick + 500) { snd_pcm_sframes_t delay_frames; unsigned long delay_us; diff --git a/rdpsnd_libao.c b/rdpsnd_libao.c index 3a59788..b116742 100644 --- a/rdpsnd_libao.c +++ b/rdpsnd_libao.c @@ -148,6 +148,7 @@ libao_play(void) { struct audio_packet *packet; STREAM out; + unsigned char *data; int len; static long prev_s, prev_us; int duration; @@ -172,8 +173,8 @@ libao_play(void) next_tick = rdpsnd_queue_next_tick(); len = MIN(WAVEOUTLEN, s_remaining(out)); - ao_play(o_device, (char *) out->p, len); - out->p += len; + in_uint8p(out, data, len); + ao_play(o_device, (char *) data, len); gettimeofday(&tv, NULL); @@ -182,7 +183,7 @@ libao_play(void) if (packet->tick > next_tick) next_tick += 65536; - if ((out->p == out->end) || duration > next_tick - packet->tick + 500) + if (s_check_end(out) || duration > next_tick - packet->tick + 500) { unsigned int delay_us; diff --git a/rdpsnd_oss.c b/rdpsnd_oss.c index 0705236..561e78c 100644 --- a/rdpsnd_oss.c +++ b/rdpsnd_oss.c @@ -402,6 +402,8 @@ oss_play(void) struct audio_packet *packet; ssize_t len; STREAM out; + size_t before; + const unsigned char *data; assert(dsp_fd != -1); @@ -412,9 +414,12 @@ oss_play(void) packet = rdpsnd_queue_current_packet(); out = packet->s; - len = s_remaining(out); + before = s_tell(out); - len = write(dsp_fd, out->p, (len > MAX_LEN) ? MAX_LEN : len); + len = MIN(s_remaining(out), MAX_LEN); + in_uint8p(out, data, len); + + len = write(dsp_fd, data, len); if (len == -1) { if (errno != EWOULDBLOCK) @@ -429,9 +434,10 @@ oss_play(void) dsp_broken = False; - out->p += len; + /* We might not have written everything */ + s_seek(out, before + len); - if (out->p == out->end) + if (s_check_end(out)) { int delay_bytes; unsigned long delay_us; diff --git a/rdpsnd_pulse.c b/rdpsnd_pulse.c index a763541..6fde46f 100644 --- a/rdpsnd_pulse.c +++ b/rdpsnd_pulse.c @@ -1195,8 +1195,11 @@ pulse_play(void) audio_size = MIN(s_remaining(out), avail_space); if (audio_size) { + unsigned char *data; + + in_uint8p(out, data, audio_size); if (pa_stream_write - (playback_stream, out->p, audio_size, NULL, 0, playback_seek) != 0) + (playback_stream, data, audio_size, NULL, 0, playback_seek) != 0) { err = pa_context_errno(context); logger(Sound, Error, "pulse_play(), pa_stream_write: %s", @@ -1207,9 +1210,7 @@ pulse_play(void) playback_seek = PA_SEEK_RELATIVE; } - out->p += audio_size; - - if (out->p == out->end) + if (s_check_end(out)) { ret = pa_stream_get_latency(playback_stream, &delay, NULL); if (ret != 0 && (err = pa_context_errno(context)) == PA_ERR_NODATA) @@ -1251,7 +1252,7 @@ pulse_play(void) pa_threaded_mainloop_unlock(mainloop); - if (out->p == out->end) + if (s_check_end(out)) rdpsnd_queue_next(delay); return result; diff --git a/rdpsnd_sgi.c b/rdpsnd_sgi.c index d62f12c..d1dce66 100644 --- a/rdpsnd_sgi.c +++ b/rdpsnd_sgi.c @@ -246,6 +246,7 @@ sgi_play(void) struct audio_packet *packet; ssize_t len; STREAM out; + unsigned char *data; int gf; while (1) @@ -257,11 +258,11 @@ sgi_play(void) out = packet->s; len = s_remaining(out); + in_uint8p(out, data, len); - alWriteFrames(output_port, out->p, len / combinedFrameSize); + alWriteFrames(output_port, data, len / combinedFrameSize); - out->p += len; - if (out->p == out->end) + if (s_check_end(out)) { gf = alGetFilled(output_port); if (gf < (4 * maxFillable / 10)) diff --git a/rdpsnd_sun.c b/rdpsnd_sun.c index 50a8fb2..b682ca5 100644 --- a/rdpsnd_sun.c +++ b/rdpsnd_sun.c @@ -411,17 +411,22 @@ sun_play(void) struct audio_packet *packet; ssize_t len; STREAM out; + size_t before; + const unsigned char *data; /* We shouldn't be called if the queue is empty, but still */ if (rdpsnd_queue_empty()) return; packet = rdpsnd_queue_current_packet(); - out = &packet->s; + out = packet->s; - len = s_remaining(out); + before = s_tell(out); - len = write(dsp_fd, out->p, (len > MAX_LEN) ? MAX_LEN : len); + len = MIN(s_remaining(out), MAX_LEN); + in_uint8p(out, data, len); + + len = write(dsp_fd, data, len); if (len == -1) { if (errno != EWOULDBLOCK) @@ -439,9 +444,10 @@ sun_play(void) dsp_broken = False; - out->p += len; + /* We might not have written everything */ + s_seek(out, before + len); - if (out->p == out->end) + if (s_check_end(out)) { audio_info_t info; uint_t delay_samples; diff --git a/scard.c b/scard.c index 9e4e807..92b984a 100644 --- a/scard.c +++ b/scard.c @@ -562,7 +562,7 @@ outBufferFinish(STREAM out, char *buffer, unsigned int length) static void outForceAlignment(STREAM out, unsigned int seed) { - SERVER_DWORD add = (seed - (out->p - out->data) % seed) % seed; + SERVER_DWORD add = (seed - s_tell(out) % seed) % seed; if (add > 0) out_uint8s(out, add); } @@ -639,7 +639,7 @@ static void inReaderName(PMEM_HANDLE * handle, STREAM in, char **destination, RD_BOOL wide) { SERVER_DWORD dataLength; - in->p += 0x08; + in_uint8s(in, 0x08); in_uint32_le(in, dataLength); inRepos(in, inString(handle, in, destination, dataLength, wide)); } @@ -717,6 +717,7 @@ TS_SCardEstablishContext(STREAM in, STREAM out) out_uint32_le(out, 0x00000004); out_uint32_le(out, hContext); outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -727,7 +728,7 @@ TS_SCardReleaseContext(STREAM in, STREAM out) MYPCSC_SCARDCONTEXT myHContext; SERVER_SCARDCONTEXT hContext; - in->p += 0x1C; + in_uint8s(in, 0x1C); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -752,6 +753,7 @@ TS_SCardReleaseContext(STREAM in, STREAM out) } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -762,7 +764,7 @@ TS_SCardIsValidContext(STREAM in, STREAM out) SERVER_SCARDCONTEXT hContext; MYPCSC_SCARDCONTEXT myHContext; - in->p += 0x1C; + in_uint8s(in, 0x1C); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -786,6 +788,7 @@ TS_SCardIsValidContext(STREAM in, STREAM out) } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -799,21 +802,21 @@ TS_SCardListReaders(STREAM in, STREAM out, RD_BOOL wide) MYPCSC_SCARDCONTEXT myHContext; SERVER_DWORD dataLength; MYPCSC_DWORD cchReaders = readerArraySize; - unsigned char *plen1, *plen2, *pend; + size_t plen1, plen2, pend; char *readers, *cur; PMEM_HANDLE lcHandle = NULL; - in->p += 0x2C; + in_uint8s(in, 0x2C); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); logger(SmartCard, Debug, "TS_SCardListReaders(), context: 0x%08x [0x%lx])", (unsigned) hContext, myHContext); - plen1 = out->p; + plen1 = s_tell(out); out_uint32_le(out, 0x00000000); /* Temp value for data length as 0x0 */ out_uint32_le(out, 0x01760650); - plen2 = out->p; + plen2 = s_tell(out); out_uint32_le(out, 0x00000000); /* Temp value for data length as 0x0 */ dataLength = 0; @@ -863,14 +866,17 @@ TS_SCardListReaders(STREAM in, STREAM out, RD_BOOL wide) dataLength += outString(out, "\0", wide); outRepos(out, dataLength); - pend = out->p; - out->p = plen1; + s_mark_end(out); + + pend = s_tell(out); + s_seek(out, plen1); out_uint32_le(out, dataLength); - out->p = plen2; + s_seek(out, plen2); out_uint32_le(out, dataLength); - out->p = pend; + s_seek(out, pend); outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -891,11 +897,11 @@ TS_SCardConnect(STREAM in, STREAM out, RD_BOOL wide) MYPCSC_DWORD dwActiveProtocol; PMEM_HANDLE lcHandle = NULL; - in->p += 0x1C; + in_uint8s(in, 0x1C); in_uint32_le(in, dwShareMode); in_uint32_le(in, dwPreferredProtocol); inReaderName(&lcHandle, in, &szReader, wide); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -966,6 +972,7 @@ TS_SCardConnect(STREAM in, STREAM out, RD_BOOL wide) out_uint32_le(out, hCard); outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -982,13 +989,13 @@ TS_SCardReconnect(STREAM in, STREAM out) SERVER_DWORD dwInitialization; MYPCSC_DWORD dwActiveProtocol; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwShareMode); in_uint32_le(in, dwPreferredProtocol); in_uint32_le(in, dwInitialization); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hContext); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hCard); @@ -1014,6 +1021,7 @@ TS_SCardReconnect(STREAM in, STREAM out) out_uint32_le(out, (SERVER_DWORD) dwActiveProtocol); outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -1027,11 +1035,11 @@ TS_SCardDisconnect(STREAM in, STREAM out) MYPCSC_SCARDHANDLE myHCard; SERVER_DWORD dwDisposition; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwDisposition); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hContext); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hCard); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -1076,6 +1084,7 @@ TS_SCardDisconnect(STREAM in, STREAM out) } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -1162,12 +1171,12 @@ TS_SCardGetStatusChange(STREAM in, STREAM out, RD_BOOL wide) long i; PMEM_HANDLE lcHandle = NULL; - in->p += 0x18; + in_uint8s(in, 0x18); in_uint32_le(in, dwTimeout); in_uint32_le(in, dwCount); - in->p += 0x08; + in_uint8s(in, 0x08); in_uint32_le(in, hContext); - in->p += 0x04; + in_uint8s(in, 0x04); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -1197,7 +1206,7 @@ TS_SCardGetStatusChange(STREAM in, STREAM out, RD_BOOL wide) { SERVER_DWORD dataLength; - in->p += 0x08; + in_uint8s(in, 0x08); in_uint32_le(in, dataLength); inRepos(in, inString(&lcHandle, in, (char **) &(cur->szReader), @@ -1259,6 +1268,7 @@ TS_SCardGetStatusChange(STREAM in, STREAM out, RD_BOOL wide) sizeof(SERVER_SCARD_READERSTATE_A) - 2 * sizeof(unsigned char *)); } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1270,7 +1280,7 @@ TS_SCardCancel(STREAM in, STREAM out) SERVER_SCARDCONTEXT hContext; MYPCSC_SCARDCONTEXT myHContext; - in->p += 0x1C; + in_uint8s(in, 0x1C); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -1289,6 +1299,7 @@ TS_SCardCancel(STREAM in, STREAM out) logger(SmartCard, Debug, "TS_SCardCancel(), success"); } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -1309,7 +1320,7 @@ TS_SCardLocateCardsByATR(STREAM in, STREAM out, RD_BOOL wide) MYPCSC_LPSCARD_READERSTATE_A myRsArray; PMEM_HANDLE lcHandle = NULL; - in->p += 0x2C; + in_uint8s(in, 0x2C); in_uint32_le(in, hContext); in_uint32_le(in, atrMaskCount); pAtrMasks = SC_xmalloc(&lcHandle, atrMaskCount * sizeof(SCARD_ATRMASK_L)); @@ -1424,6 +1435,7 @@ TS_SCardLocateCardsByATR(STREAM in, STREAM out, RD_BOOL wide) } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1435,7 +1447,7 @@ TS_SCardBeginTransaction(STREAM in, STREAM out) SERVER_SCARDCONTEXT hCard; MYPCSC_SCARDCONTEXT myHCard; - in->p += 0x30; + in_uint8s(in, 0x30); in_uint32_le(in, hCard); myHCard = _scard_handle_list_get_pcsc_handle(hCard); logger(SmartCard, Debug, "TS_SCardBeginTransaction(), hcard: 0x%08x [0x%lx])", @@ -1451,6 +1463,7 @@ TS_SCardBeginTransaction(STREAM in, STREAM out) logger(SmartCard, Debug, "TS_SCardBeginTransaction(), success"); } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -1462,9 +1475,9 @@ TS_SCardEndTransaction(STREAM in, STREAM out) MYPCSC_SCARDCONTEXT myHCard; SERVER_DWORD dwDisposition = 0; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwDisposition); - in->p += 0x0C; + in_uint8s(in, 0x0C); in_uint32_le(in, hCard); myHCard = _scard_handle_list_get_pcsc_handle(hCard); @@ -1483,6 +1496,7 @@ TS_SCardEndTransaction(STREAM in, STREAM out) logger(SmartCard, Debug, "TS_SCardEndTransaction(), success"); } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -1531,9 +1545,9 @@ TS_SCardTransmit(STREAM in, STREAM out, uint32 srv_buf_len) PMEM_HANDLE lcHandle = NULL; - in->p += 0x14; + in_uint8s(in, 0x14); in_uint32_le(in, map[0]); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, map[1]); pioSendPci = SC_xmalloc(&lcHandle, sizeof(SERVER_SCARD_IO_REQUEST)); if (!pioSendPci) @@ -1551,7 +1565,7 @@ TS_SCardTransmit(STREAM in, STREAM out, uint32 srv_buf_len) if (srv_buf_len <= cbRecvLength) cbRecvLength = srv_buf_len; - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hCard); myHCard = _scard_handle_list_get_pcsc_handle(hCard); @@ -1695,6 +1709,7 @@ TS_SCardTransmit(STREAM in, STREAM out, uint32 srv_buf_len) outBufferFinish(out, (char *) recvBuf, cbRecvLength); } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1712,12 +1727,12 @@ TS_SCardStatus(STREAM in, STREAM out, RD_BOOL wide) char *readerName; unsigned char *atr; - in->p += 0x24; + in_uint8s(in, 0x24); in_uint32_le(in, dwReaderLen); in_uint32_le(in, dwAtrLen); - in->p += 0x0C; + in_uint8s(in, 0x0C); in_uint32_le(in, hCard); - in->p += 0x04; + in_uint8s(in, 0x04); myHCard = _scard_handle_list_get_pcsc_handle(hCard); logger(SmartCard, Debug, "TS_SCardStatus(), hcard: 0x%08x [0x%08lx], reader len: %d bytes, atr len: %d bytes", @@ -1790,7 +1805,7 @@ TS_SCardStatus(STREAM in, STREAM out, RD_BOOL wide) else dwState = 0x00000000; - void *p_len1 = out->p; + size_t p_len1 = s_tell(out); out_uint32_le(out, dwReaderLen); out_uint32_le(out, 0x00020000); out_uint32_le(out, dwState); @@ -1802,19 +1817,21 @@ TS_SCardStatus(STREAM in, STREAM out, RD_BOOL wide) } out_uint32_le(out, dwAtrLen); - void *p_len2 = out->p; + size_t p_len2 = s_tell(out); out_uint32_le(out, dwReaderLen); dataLength = outString(out, readerName, wide); dataLength += outString(out, "\0", wide); outRepos(out, dataLength); - void *psave = out->p; - out->p = p_len1; + s_mark_end(out); + size_t psave = s_tell(out); + s_seek(out, p_len1); out_uint32_le(out, dataLength); - out->p = p_len2; + s_seek(out, p_len2); out_uint32_le(out, dataLength); - out->p = psave; + s_seek(out, psave); } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1831,11 +1848,11 @@ TS_SCardState(STREAM in, STREAM out) char *readerName; unsigned char *atr; - in->p += 0x24; + in_uint8s(in, 0x24); in_uint32_le(in, dwAtrLen); - in->p += 0x0C; + in_uint8s(in, 0x0C); in_uint32_le(in, hCard); - in->p += 0x04; + in_uint8s(in, 0x04); myHCard = _scard_handle_list_get_pcsc_handle(hCard); logger(SmartCard, Debug, "TS_SCardState(), hcard: 0x%08x [0x%08lx], atrlen: %d bytes", @@ -1905,6 +1922,7 @@ TS_SCardState(STREAM in, STREAM out) outRepos(out, dwAtrLen); } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1926,9 +1944,9 @@ TS_SCardListReaderGroups(STREAM in, STREAM out) char *szGroups; PMEM_HANDLE lcHandle = NULL; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwGroups); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hContext); myHContext = _scard_handle_list_get_pcsc_handle(hContext); @@ -1974,6 +1992,7 @@ TS_SCardListReaderGroups(STREAM in, STREAM out) out_uint32_le(out, 0x00000000); outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -1990,11 +2009,11 @@ TS_SCardGetAttrib(STREAM in, STREAM out) unsigned char *pbAttr; PMEM_HANDLE lcHandle = NULL; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwAttrId); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, dwAttrLen); - in->p += 0x0C; + in_uint8s(in, 0x0C); in_uint32_le(in, hCard); myHCard = _scard_handle_list_get_pcsc_handle(hCard); @@ -2044,6 +2063,7 @@ TS_SCardGetAttrib(STREAM in, STREAM out) out_uint32_le(out, 0x00000000); } outForceAlignment(out, 8); + s_mark_end(out); return rv; } @@ -2060,11 +2080,11 @@ TS_SCardSetAttrib(STREAM in, STREAM out) unsigned char *pbAttr; PMEM_HANDLE lcHandle = NULL; - in->p += 0x20; + in_uint8s(in, 0x20); in_uint32_le(in, dwAttrId); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, dwAttrLen); - in->p += 0x0C; + in_uint8s(in, 0x0C); in_uint32_le(in, hCard); myHCard = scHandleToMyPCSC(hCard); @@ -2098,6 +2118,7 @@ TS_SCardSetAttrib(STREAM in, STREAM out) out_uint32_le(out, 0x00000000); out_uint32_le(out, 0x00000000); outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -2123,18 +2144,18 @@ TS_SCardControl(STREAM in, STREAM out) pInBuffer = NULL; pOutBuffer = NULL; - in->p += 0x14; + in_uint8s(in, 0x14); in_uint32_le(in, map[0]); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, map[1]); in_uint32_le(in, dwControlCode); in_uint32_le(in, nInBufferSize); in_uint32_le(in, map[2]); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, nOutBufferSize); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hContext); - in->p += 0x04; + in_uint8s(in, 0x04); in_uint32_le(in, hCard); if (map[2] & INPUT_LINKED) { @@ -2241,6 +2262,7 @@ TS_SCardControl(STREAM in, STREAM out) } outForceAlignment(out, 8); + s_mark_end(out); SC_xfreeallmemory(&lcHandle); return rv; } @@ -2260,7 +2282,7 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out, { UNUSED(handle); SERVER_DWORD Result = 0x00000000; - unsigned char *psize, *pend, *pStatusCode; + size_t psize, pend, pStatusCode; SERVER_DWORD addToEnd = 0; /* Processing request */ @@ -2270,11 +2292,11 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out, /* Set CommonTypeHeader (MS-RPCE 2.2.6.1) */ out_uint32_le(out, 0x00081001); /* Header lines */ out_uint32_le(out, 0xCCCCCCCC); - psize = out->p; + psize = s_tell(out); /* Set PrivateTypeHeader (MS-RPCE 2.2.6.2) */ out_uint32_le(out, 0x00000000); /* Size of data portion */ out_uint32_le(out, 0x00000000); /* Zero bytes (may be useful) */ - pStatusCode = out->p; + pStatusCode = s_tell(out); out_uint32_le(out, 0x00000000); /* Status Code */ switch (request) @@ -2410,22 +2432,25 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out, #if 0 out_uint32_le(out, 0x00000000); #endif + s_mark_end(out); + /* Setting modified variables */ - pend = out->p; + pend = s_tell(out); /* setting data size */ - out->p = psize; + s_seek(out, psize); out_uint32_le(out, pend - psize - 16); /* setting status code */ - out->p = pStatusCode; + s_seek(out, pStatusCode); out_uint32_le(out, Result); /* finish */ - out->p = pend; + s_seek(out, pend); /* TODO: Check MS-RPCE 2.2.6.2 for alignment requirements (IIRC length must be integral multiple of 8) */ addToEnd = (pend - pStatusCode) % 16; if (addToEnd < 16 && addToEnd > 0) { out_uint8s(out, addToEnd); + s_mark_end(out); } if (Result == SCARD_E_INSUFFICIENT_BUFFER) return RD_STATUS_BUFFER_TOO_SMALL; @@ -2574,13 +2599,18 @@ SC_deviceControl(PSCThreadData data) RD_NTSTATUS status; size_t buffer_len = 0; status = scard_device_control(data->handle, data->request, data->in, data->out, data->srv_buf_len); - buffer_len = (size_t) data->out->p - (size_t) data->out->data; + buffer_len = s_tell(data->out); /* if iorequest belongs to another epoch, don't send response back to server due to it's considered as abandoned. */ if (data->epoch == curEpoch) - rdpdr_send_completion(data->device, data->id, status, buffer_len, data->out->data, buffer_len); + { + uint8 *buf; + s_seek(data->out, 0); + in_uint8p(data->out, buf, buffer_len); + rdpdr_send_completion(data->device, data->id, status, buffer_len, buf, buffer_len); + } SC_destroyThreadData(data); } diff --git a/seamless.c b/seamless.c index f5cc7a4..289b30a 100644 --- a/seamless.c +++ b/seamless.c @@ -380,7 +380,8 @@ seamless_process(STREAM s) pkglen = s_remaining(s); /* str_handle_lines requires null terminated strings */ buf = xmalloc(pkglen + 1); - STRNCPY(buf, (char *) s->p, pkglen + 1); + in_uint8a(s, buf, pkglen); + buf[pkglen] = '\0'; str_handle_lines(buf, &seamless_rest, seamless_line_handler, NULL); xfree(buf); diff --git a/secure.c b/secure.c index 8c5dd42..eb7c129 100644 --- a/secure.c +++ b/secure.c @@ -351,10 +351,12 @@ sec_send_to_channel(STREAM s, uint32 flags, uint16 channel) if (flags & SEC_ENCRYPT) { + unsigned char *data; flags &= ~SEC_ENCRYPT; datalen = s_remaining(s) - 8; - sec_sign(s->p, 8, g_sec_sign_key, g_rc4_key_len, s->p + 8, datalen); - sec_encrypt(s->p + 8, datalen); + inout_uint8p(s, data, datalen + 8); + sec_sign(data, 8, g_sec_sign_key, g_rc4_key_len, data + 8, datalen); + sec_encrypt(data + 8, datalen); } mcs_send_to_channel(s, channel); @@ -587,7 +589,7 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, RDSSL_CERT *cacert, *server_cert; RDSSL_RKEY *server_public_key; uint16 tag, length; - uint8 *next_tag, *end; + size_t next_tag; logger(Protocol, Debug, "%s()", __func__); @@ -613,10 +615,9 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, in_uint8p(s, *server_random, random_len); /* RSA info */ - end = s->p + rsa_info_len; - if (end > s->end) + if (!s_check_rem(s, rsa_info_len)) { - logger(Protocol, Error, "sec_parse_crypt_info(), end > s->end"); + logger(Protocol, Error, "sec_parse_crypt_info(), !s_check_rem(s, rsa_info_len)"); return False; } @@ -627,12 +628,12 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, "sec_parse_crypt_info(), We're going for the RDP4-style encryption"); in_uint8s(s, 8); /* unknown */ - while (s->p < end) + while (!s_check_end(s)) { in_uint16_le(s, tag); in_uint16_le(s, length); - next_tag = s->p + length; + next_tag = s_tell(s) + length; switch (tag) { @@ -663,12 +664,13 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, tag); } - s->p = next_tag; + s_seek(s, next_tag); } } else { uint32 certcount; + unsigned char *certdata; logger(Protocol, Debug, "sec_parse_crypt_info(), We're going for the RDP5-style encryption"); @@ -683,10 +685,11 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, { /* ignore all the certificates between the root and the signing CA */ uint32 ignorelen; RDSSL_CERT *ignorecert; + unsigned char *ignoredata; in_uint32_le(s, ignorelen); - ignorecert = rdssl_cert_read(s->p, ignorelen); - in_uint8s(s, ignorelen); + in_uint8p(s, ignoredata, ignorelen); + ignorecert = rdssl_cert_read(ignoredata, ignorelen); if (ignorecert == NULL) { /* XXX: error out? */ logger(Protocol, Error, @@ -706,10 +709,10 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, http://www.cs.auckland.ac.nz/~pgut001/pubs/x509guide.txt */ in_uint32_le(s, cacert_len); + in_uint8p(s, certdata, cacert_len); logger(Protocol, Debug, "sec_parse_crypt_info(), server CA Certificate length is %d", cacert_len); - cacert = rdssl_cert_read(s->p, cacert_len); - in_uint8s(s, cacert_len); + cacert = rdssl_cert_read(certdata, cacert_len); if (NULL == cacert) { logger(Protocol, Error, @@ -717,10 +720,10 @@ sec_parse_crypt_info(STREAM s, uint32 * rc4_key_size, return False; } in_uint32_le(s, cert_len); + in_uint8p(s, certdata, cert_len); logger(Protocol, Debug, "sec_parse_crypt_info(), certificate length is %d", cert_len); - server_cert = rdssl_cert_read(s->p, cert_len); - in_uint8s(s, cert_len); + server_cert = rdssl_cert_read(certdata, cert_len); if (NULL == server_cert) { rdssl_cert_free(cacert); @@ -814,7 +817,7 @@ void sec_process_mcs_data(STREAM s) { uint16 tag, length; - uint8 *next_tag; + size_t next_tag; uint8 len; in_uint8s(s, 21); /* header (T.124 ConferenceCreateResponse) */ @@ -823,7 +826,7 @@ sec_process_mcs_data(STREAM s) in_uint8(s, len); logger(Protocol, Debug, "%s()", __func__); - while (s->p < s->end) + while (!s_check_end(s)) { in_uint16_le(s, tag); in_uint16_le(s, length); @@ -831,7 +834,7 @@ sec_process_mcs_data(STREAM s) if (length <= 4) return; - next_tag = s->p + length - 4; + next_tag = s_tell(s) + length - 4; switch (tag) { @@ -856,7 +859,7 @@ sec_process_mcs_data(STREAM s) logger(Protocol, Warning, "Unhandled response tag 0x%x", tag); } - s->p = next_tag; + s_seek(s, next_tag); } } @@ -869,6 +872,8 @@ sec_recv(RD_BOOL * is_fastpath) uint16 channel; STREAM s; struct stream packet; + size_t data_offset; + unsigned char *data; while ((s = mcs_recv(&channel, is_fastpath, &fastpath_hdr)) != NULL) { @@ -886,19 +891,29 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s_remaining(s)); + + data_offset = s_tell(s); + + inout_uint8p(s, data, s_remaining(s)); + sec_decrypt(data, s_remaining(s)); + + s_seek(s, data_offset); } return s; } if (g_encryption || (!g_licence_issued && !g_licence_error_result)) { + data_offset = s_tell(s); + /* TS_SECURITY_HEADER */ in_uint16_le(s, sec_flags); in_uint8s(s, 2); /* skip sec_flags_hi */ if (g_encryption) { + data_offset = s_tell(s); + if (sec_flags & SEC_ENCRYPT) { if (!s_check_rem(s, 8)) { @@ -906,11 +921,16 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s_remaining(s)); + + data_offset = s_tell(s); + + inout_uint8p(s, data, s_remaining(s)); + sec_decrypt(data, s_remaining(s)); } if (sec_flags & SEC_LICENSE_PKT) { + s_seek(s, data_offset); licence_process(s); continue; } @@ -924,10 +944,14 @@ sec_recv(RD_BOOL * is_fastpath) } in_uint8s(s, 8); /* signature */ - sec_decrypt(s->p, s_remaining(s)); + + data_offset = s_tell(s); + + inout_uint8p(s, data, s_remaining(s)); + sec_decrypt(data, s_remaining(s)); /* Check for a redirect packet, starts with 00 04 */ - if (s->p[0] == 0 && s->p[1] == 4) + if (data[0] == 0 && data[1] == 4) { /* for some reason the PDU and the length seem to be swapped. This isn't good, but we're going to do a byte for byte @@ -935,17 +959,17 @@ sec_recv(RD_BOOL * is_fastpath) where XX YY is the little endian length. We're going to use 04 00 as the PDU type, so after our swap this will look like: XX YY 04 00 */ - swapbyte = s->p[0]; - s->p[0] = s->p[2]; - s->p[2] = swapbyte; + swapbyte = data[0]; + data[0] = data[2]; + data[2] = swapbyte; - swapbyte = s->p[1]; - s->p[1] = s->p[3]; - s->p[3] = swapbyte; + swapbyte = data[1]; + data[1] = data[3]; + data[3] = swapbyte; - swapbyte = s->p[2]; - s->p[2] = s->p[3]; - s->p[3] = swapbyte; + swapbyte = data[2]; + data[2] = data[3]; + data[3] = swapbyte; } } } @@ -956,8 +980,9 @@ sec_recv(RD_BOOL * is_fastpath) licence_process(s); continue; } - s->p -= 4; } + + s_seek(s, data_offset); } if (channel != MCS_GLOBAL_CHANNEL) diff --git a/stream.h b/stream.h index 5e7464b..b2400c5 100644 --- a/stream.h +++ b/stream.h @@ -152,6 +152,9 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); /* Copy the entire STREAM v (ignoring offsets) in to STREAM s */ #define out_stream(s, v) out_uint8a(s, (v)->data, s_length((v))) +/* Return a pointer in v to manually modify n bytes of STREAM s in place */ +#define inout_uint8p(s,v,n) { s_assert_r(s, n); s_assert_w(s, n); v = (s)->p; (s)->p += n; } + #define next_be(s,v) { s_assert_r(s, 1); v = ((v) << 8) + *((s)->p++); } diff --git a/tcp.c b/tcp.c index 8634631..3f8b112 100644 --- a/tcp.c +++ b/tcp.c @@ -109,8 +109,9 @@ tcp_init(uint32 maxlen) void tcp_send(STREAM s) { - int length = s->end - s->data; - int sent, total = 0; + size_t before; + int length, sent; + unsigned char *data; if (g_network_error == True) return; @@ -119,10 +120,16 @@ tcp_send(STREAM s) scard_lock(SCARD_LOCK_TCP); #endif - while (total < length) + s_seek(s, 0); + + while (!s_check_end(s)) { + before = s_tell(s); + length = s_remaining(s); + in_uint8p(s, data, length); + if (g_ssl_initialized) { - sent = gnutls_record_send(g_tls_session, s->data + total, length - total); + sent = gnutls_record_send(g_tls_session, data, length); if (sent <= 0) { if (gnutls_error_is_fatal(sent)) { #ifdef WITH_SCARD @@ -139,7 +146,7 @@ tcp_send(STREAM s) } else { - sent = send(g_sock, s->data + total, length - total, 0); + sent = send(g_sock, data, length, 0); if (sent <= 0) { if (sent == -1 && TCP_BLOCKS) @@ -159,7 +166,9 @@ tcp_send(STREAM s) } } } - total += sent; + + /* Everything might not have been sent */ + s_seek(s, before + sent); } #ifdef WITH_SCARD scard_unlock(SCARD_LOCK_TCP); @@ -170,7 +179,8 @@ tcp_send(STREAM s) STREAM tcp_recv(STREAM s, uint32 length) { - uint32 new_length, end_offset, p_offset; + size_t before; + unsigned char *data; int rcvd = 0; if (g_network_error == True) @@ -179,27 +189,14 @@ tcp_recv(STREAM s, uint32 length) if (s == NULL) { /* read into "new" stream */ - if (length > g_in.size) - { - g_in.data = (uint8 *) xrealloc(g_in.data, length); - g_in.size = length; - } - g_in.end = g_in.p = g_in.data; + s_realloc(&g_in, length); + s_reset(&g_in); s = &g_in; } else { /* append to existing stream */ - new_length = (s->end - s->data) + length; - if (new_length > s->size) - { - p_offset = s->p - s->data; - end_offset = s->end - s->data; - s->data = (uint8 *) xrealloc(s->data, new_length); - s->size = new_length; - s->p = s->data + p_offset; - s->end = s->data + end_offset; - } + s_realloc(s, s_length(s) + length); } while (length > 0) @@ -215,8 +212,15 @@ tcp_recv(STREAM s, uint32 length) return NULL; } + before = s_tell(s); + s_seek(s, s_length(s)); + + out_uint8p(s, data, length); + + s_seek(s, before); + if (g_ssl_initialized) { - rcvd = gnutls_record_recv(g_tls_session, s->end, length); + rcvd = gnutls_record_recv(g_tls_session, data, length); if (rcvd < 0) { if (gnutls_error_is_fatal(rcvd)) { @@ -231,7 +235,7 @@ tcp_recv(STREAM s, uint32 length) } else { - rcvd = recv(g_sock, s->end, length, 0); + rcvd = recv(g_sock, data, length, 0); if (rcvd < 0) { if (rcvd == -1 && TCP_BLOCKS) @@ -253,6 +257,7 @@ tcp_recv(STREAM s, uint32 length) } } + // FIXME: Should probably have a macro for this s->end += rcvd; length -= rcvd; } From f19c21d7d13210f507a4a0b67a8318d6e8343451 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 12 Apr 2019 13:56:03 +0200 Subject: [PATCH 19/21] Fix memory leak in disk redirection We kept allocating a data buffer for this stream in each call, but never freeing it. --- disk.c | 57 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/disk.c b/disk.c index 5ebede6..2d5e246 100644 --- a/disk.c +++ b/disk.c @@ -1131,10 +1131,7 @@ disk_query_volume_information(RD_NTHANDLE handle, uint32 info_class, STREAM out) struct STATFS_T stat_fs; struct fileinfo *pfinfo; FsInfoType *fsinfo; - struct stream stmp; - - memset(&stmp, 0, sizeof(stmp)); - s_realloc(&stmp, PATH_MAX * 4); + STREAM stmp; logger(Disk, Debug, "disk_query_volume_information(handle=0x%x, info_class=0x%x)", handle, info_class); @@ -1153,16 +1150,17 @@ disk_query_volume_information(RD_NTHANDLE handle, uint32 info_class, STREAM out) switch (info_class) { case FileFsVolumeInformation: - s_reset(&stmp); - out_utf16s(&stmp, fsinfo->label); - s_mark_end(&stmp); + stmp = s_alloc(PATH_MAX * 4); + out_utf16s(stmp, fsinfo->label); + s_mark_end(stmp); out_uint32_le(out, 0); /* volume creation time low */ out_uint32_le(out, 0); /* volume creation time high */ out_uint32_le(out, fsinfo->serial); /* serial */ - out_uint32_le(out, s_length(&stmp)); /* length of string */ + out_uint32_le(out, s_length(stmp)); /* length of string */ out_uint8(out, 0); /* support objects? */ - out_stream(out, &stmp); /* fsinfo->label string */ + out_stream(out, stmp); /* fsinfo->label string */ + s_free(stmp); break; case FileFsSizeInformation: @@ -1183,15 +1181,16 @@ disk_query_volume_information(RD_NTHANDLE handle, uint32 info_class, STREAM out) break; case FileFsAttributeInformation: - s_reset(&stmp); - out_utf16s_no_eos(&stmp, fsinfo->type); - s_mark_end(&stmp); + stmp = s_alloc(PATH_MAX * 4); + out_utf16s_no_eos(stmp, fsinfo->type); + s_mark_end(stmp); out_uint32_le(out, FS_CASE_SENSITIVE | FS_CASE_IS_PRESERVED); /* fs attributes */ out_uint32_le(out, F_NAMELEN(stat_fs)); /* max length of filename */ - out_uint32_le(out, s_length(&stmp)); /* length of fsinfo->type string */ - out_stream(out, &stmp); /* fsinfo->typ string */ + out_uint32_le(out, s_length(stmp)); /* length of fsinfo->type string */ + out_stream(out, stmp); /* fsinfo->typ string */ + s_free(stmp); break; case FileFsLabelInformation: @@ -1218,7 +1217,7 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA struct dirent *pdirent; struct stat filestat; struct fileinfo *pfinfo; - struct stream stmp; + STREAM stmp; logger(Disk, Debug, "disk_query_directory(handle=0x%x, info_class=0x%x, pattern=%s, ...)", handle, info_class, pattern); @@ -1228,9 +1227,6 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA dirname = pfinfo->path; file_attributes = 0; - memset(&stmp, 0, sizeof(stmp)); - s_realloc(&stmp, PATH_MAX * 4); - switch (info_class) { case FileBothDirectoryInformation: @@ -1299,9 +1295,9 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA } // Write entry name as utf16 into stmp - s_reset(&stmp); - out_utf16s_no_eos(&stmp, pdirent->d_name); - s_mark_end(&stmp); + stmp = s_alloc(PATH_MAX * 4); + out_utf16s_no_eos(stmp, pdirent->d_name); + s_mark_end(stmp); switch (info_class) { @@ -1327,11 +1323,11 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA out_uint64_le(out, filestat.st_size); /* filesize */ out_uint64_le(out, filestat.st_size); /* filesize */ out_uint32_le(out, file_attributes); /* FileAttributes */ - out_uint32_le(out, s_length(&stmp)); /* length of dir entry name string */ + out_uint32_le(out, s_length(stmp)); /* length of dir entry name string */ out_uint32_le(out, 0); /* EaSize */ out_uint8(out, 0); /* ShortNameLength */ out_uint8s(out, 24); /* ShortName (8.3 name) */ - out_stream(out, &stmp); /* dir entry name string */ + out_stream(out, stmp); /* dir entry name string */ break; @@ -1357,8 +1353,8 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA out_uint64_le(out, filestat.st_size); /* filesize */ out_uint64_le(out, filestat.st_size); /* filesize */ out_uint32_le(out, file_attributes); - out_uint32_le(out, s_length(&stmp)); /* dir entry name string length */ - out_stream(out, &stmp); /* dir entry name */ + out_uint32_le(out, s_length(stmp)); /* dir entry name string length */ + out_stream(out, stmp); /* dir entry name */ break; @@ -1384,16 +1380,16 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA out_uint64_le(out, filestat.st_size); /* filesize */ out_uint64_le(out, filestat.st_size); /* filesize */ out_uint32_le(out, file_attributes); - out_uint32_le(out, s_length(&stmp)); /* dir entry name string length */ + out_uint32_le(out, s_length(stmp)); /* dir entry name string length */ out_uint32_le(out, 0); /* EaSize */ - out_stream(out, &stmp); /* dir entry name */ + out_stream(out, stmp); /* dir entry name */ break; case FileNamesInformation: - out_uint32_le(out, s_length(&stmp)); /* dir entry name string length */ - out_stream(out, &stmp); /* dir entry name */ + out_uint32_le(out, s_length(stmp)); /* dir entry name string length */ + out_stream(out, stmp); /* dir entry name */ break; @@ -1401,9 +1397,12 @@ disk_query_directory(RD_NTHANDLE handle, uint32 info_class, char *pattern, STREA logger(Disk, Warning, "disk_query_directory(), unhandled directory info class 0x%x", info_class); + s_free(stmp); return RD_STATUS_INVALID_PARAMETER; } + s_free(stmp); + return RD_STATUS_SUCCESS; } From 77758c3c18baea79f889cff0382a7603a8321e80 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 15 Apr 2019 13:07:43 +0200 Subject: [PATCH 20/21] Handle empty unicode strings from server --- rdp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rdp.c b/rdp.c index b938c2f..8ac0d28 100644 --- a/rdp.c +++ b/rdp.c @@ -310,6 +310,17 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) abort(); } + /* Corner case. We still want to return a null terminated string... */ + if (in_len == 0) { + if (*string == NULL) + { + *string = xmalloc(1); + } + **string = '\0'; + *str_size = 0; + return; + } + if (!s_check_rem(s, in_len)) { rdp_protocol_error("consume of unicode data from stream would overrun", &packet); @@ -358,6 +369,7 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size) abort(); } + /* Always force the last byte to be a null */ *pout = 0; if (*string) From a33814c4782fab5530de43d922b4a2bdd2739e8b Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 6 May 2019 14:31:23 +0200 Subject: [PATCH 21/21] Add comments for STREAM macros Add some short descriptions that should hopefully make it easier to understand what all these macros do. --- stream.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/stream.h b/stream.h index b2400c5..d3722ea 100644 --- a/stream.h +++ b/stream.h @@ -59,8 +59,11 @@ void out_utf16s_no_eos(STREAM s, const char *string); size_t in_ansi_string(STREAM s, char *string, size_t len); +/* Store current offset as header h and skip n bytes */ #define s_push_layer(s,h,n) { (s)->h = (s)->p; (s)->p += n; } +/* Set header h as current offset */ #define s_pop_layer(s,h) (s)->p = (s)->h; +/* Mark current offset as end of readable data */ #define s_mark_end(s) (s)->end = (s)->p; /* Return current read offset in the STREAM */ #define s_tell(s) (size_t)((s)->p - (s)->data) @@ -68,15 +71,20 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define s_seek(s,o) (s)->p = (s)->data; s_assert_r(s,o); (s)->p += o; /* Returns number of bytes that can still be read from STREAM */ #define s_remaining(s) (size_t)((s)->end - (s)->p) +/* True if at least n bytes can still be read */ #define s_check_rem(s,n) (((s)->p <= (s)->end) && ((size_t)n <= s_remaining(s))) +/* True if all data has been read */ #define s_check_end(s) ((s)->p == (s)->end) +/* Return the total number of bytes that can be read */ #define s_length(s) ((s)->end - (s)->data) +/* Return the number of bytes that can still be written */ #define s_left(s) ((s)->size - (size_t)((s)->p - (s)->data)) /* Verify that there is enough data/space before accessing a STREAM */ #define s_assert_r(s,n) { if (!s_check_rem(s, n)) rdp_protocol_error( "unexpected stream overrun", s); } #define s_assert_w(s,n) { if (s_left(s) < (size_t)n) { logger(Core, Error, "%s:%d: %s(), %s", __FILE__, __LINE__, __func__, "unexpected stream overrun"); exit(0); } } +/* Read/write an unsigned integer in little-endian order */ #if defined(L_ENDIAN) && !defined(NEED_ALIGN) #define in_uint16_le(s,v) { s_assert_r(s, 2); v = *(uint16 *)((s)->p); (s)->p += 2; } #define in_uint32_le(s,v) { s_assert_r(s, 4); v = *(uint32 *)((s)->p); (s)->p += 4; } @@ -97,6 +105,7 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #endif +/* Read/write an unsigned integer in big-endian order */ #if defined(B_ENDIAN) && !defined(NEED_ALIGN) #define in_uint16_be(s,v) { s_assert_r(s, 2); v = *(uint16 *)((s)->p); (s)->p += 2; } #define in_uint32_be(s,v) { s_assert_r(s, 4); v = *(uint32 *)((s)->p); (s)->p += 4; } @@ -132,17 +141,21 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); #define out_uint64(s,v) out_uint64_le(s,v) #endif +/* Read a single unsigned byte in v from STREAM s */ #define in_uint8(s,v) { s_assert_r(s, 1); v = *((s)->p++); } /* Return a pointer in v to manually read n bytes from STREAM s */ #define in_uint8p(s,v,n) { s_assert_r(s, n); v = (s)->p; (s)->p += n; } /* Copy n bytes from STREAM s in to array v */ #define in_uint8a(s,v,n) { s_assert_r(s, n); memcpy(v,(s)->p,n); (s)->p += n; } +/* Skip reading n bytes in STREAM s */ #define in_uint8s(s,n) { s_assert_r(s, n); (s)->p += n; } +/* Write a single unsigned byte from v to STREAM s */ #define out_uint8(s,v) { s_assert_w(s, 1); *((s)->p++) = v; } /* Return a pointer in v to manually fill in n bytes in STREAM s */ #define out_uint8p(s,v,n) { s_assert_w(s, n); v = (s)->p; (s)->p += n; } /* Copy n bytes from array v in to STREAM s */ #define out_uint8a(s,v,n) { s_assert_w(s, n); memcpy((s)->p,v,n); (s)->p += n; } +/* Fill n bytes with 0:s in STREAM s */ #define out_uint8s(s,n) { s_assert_w(s, n); memset((s)->p,0,n); (s)->p += n; } /* Copy n bytes from STREAM s in to STREAM v */ @@ -155,6 +168,7 @@ size_t in_ansi_string(STREAM s, char *string, size_t len); /* Return a pointer in v to manually modify n bytes of STREAM s in place */ #define inout_uint8p(s,v,n) { s_assert_r(s, n); s_assert_w(s, n); v = (s)->p; (s)->p += n; } +/* Read one more byte of an unsigned big-endian integer */ #define next_be(s,v) { s_assert_r(s, 1); v = ((v) << 8) + *((s)->p++); }