From 98f0c271fbf3197b0831ebb2695fb364e4e7745a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 4 May 2020 17:44:15 -0400 Subject: [PATCH] GUACAMOLE-1059: Add explanatory comments and additional logging. --- .../rdpdr/rdpdr-fs-messages-file-info.c | 8 ++- .../rdp/channels/rdpdr/rdpdr-fs-messages.c | 52 ++++++++++++++++--- .../rdp/channels/rdpdr/rdpdr-messages.c | 36 +++++++++++-- .../rdp/channels/rdpdr/rdpdr-printer.c | 7 ++- src/protocols/rdp/channels/rdpdr/rdpdr.c | 10 +++- .../rdp/channels/rdpsnd/rdpsnd-messages.c | 35 +++++++++++-- src/protocols/rdp/channels/rdpsnd/rdpsnd.c | 16 ++++-- .../rdp/plugins/guacai/guacai-messages.c | 23 ++++++-- src/protocols/rdp/plugins/guacai/guacai.c | 7 ++- 9 files changed, 162 insertions(+), 32 deletions(-) diff --git a/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages-file-info.c b/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages-file-info.c index 52201a27..d5282ec1 100644 --- a/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages-file-info.c +++ b/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages-file-info.c @@ -136,9 +136,13 @@ void guac_rdpdr_fs_process_set_rename_info(guac_rdp_common_svc* svc, char destination_path[GUAC_RDP_FS_MAX_PATH]; /* Check stream size prior to reading. */ - if (Stream_GetRemainingLength(input_stream) < 6) + if (Stream_GetRemainingLength(input_stream) < 6) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File Stream does not " + "contain the required number of bytes. File sharing may not " + "work as expected."); return; - + } + /* Read structure */ Stream_Seek_UINT8(input_stream); /* ReplaceIfExists */ Stream_Seek_UINT8(input_stream); /* RootDirectory */ diff --git a/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages.c b/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages.c index 0e9581cc..25e1e158 100644 --- a/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages.c +++ b/src/protocols/rdp/channels/rdpdr/rdpdr-fs-messages.c @@ -49,8 +49,12 @@ void guac_rdpdr_fs_process_create(guac_rdp_common_svc* svc, char path[GUAC_RDP_FS_MAX_PATH]; /* Check remaining stream data prior to reading. */ - if (Stream_GetRemainingLength(input_stream) < 32) + if (Stream_GetRemainingLength(input_stream) < 32) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } /* Read "create" information */ Stream_Read_UINT32(input_stream, desired_access); @@ -128,8 +132,12 @@ void guac_rdpdr_fs_process_read(guac_rdp_common_svc* svc, wStream* output_stream; /* Check remaining bytes before reading stream. */ - if (Stream_GetRemainingLength(input_stream) < 12) + if (Stream_GetRemainingLength(input_stream) < 12) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File Stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } /* Read packet */ Stream_Read_UINT32(input_stream, length); @@ -181,8 +189,12 @@ void guac_rdpdr_fs_process_write(guac_rdp_common_svc* svc, wStream* output_stream; /* Check remaining length. */ - if (Stream_GetRemainingLength(input_stream) < 32) + if (Stream_GetRemainingLength(input_stream) < 32) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File Stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } /* Read packet */ Stream_Read_UINT32(input_stream, length); @@ -257,8 +269,12 @@ void guac_rdpdr_fs_process_volume_info(guac_rdp_common_svc* svc, int fs_information_class; /* Check remaining length */ - if (Stream_GetRemainingLength(input_stream) < 4) + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File Stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } Stream_Read_UINT32(input_stream, fs_information_class); @@ -299,8 +315,12 @@ void guac_rdpdr_fs_process_file_info(guac_rdp_common_svc* svc, int fs_information_class; /* Check remaining length */ - if (Stream_GetRemainingLength(input_stream) < 4) + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File Stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } Stream_Read_UINT32(input_stream, fs_information_class); @@ -349,8 +369,12 @@ void guac_rdpdr_fs_process_set_file_info(guac_rdp_common_svc* svc, int length; /* Check remaining length */ - if (Stream_GetRemainingLength(input_stream) < 32) + if (Stream_GetRemainingLength(input_stream) < 32) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } Stream_Read_UINT32(input_stream, fs_information_class); Stream_Read_UINT32(input_stream, length); /* Length */ @@ -430,8 +454,12 @@ void guac_rdpdr_fs_process_query_directory(guac_rdp_common_svc* svc, if (file == NULL) return; - if (Stream_GetRemainingLength(input_stream) < 9) + if (Stream_GetRemainingLength(input_stream) < 9) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File stream does not " + "contain the expected number of bytes. File sharing may not " + "work as expected."); return; + } /* Read main header */ Stream_Read_UINT32(input_stream, fs_information_class); @@ -441,8 +469,16 @@ void guac_rdpdr_fs_process_query_directory(guac_rdp_common_svc* svc, /* If this is the first query, the path is included after padding */ if (initial_query) { - if (Stream_GetRemainingLength(input_stream) < 23) + /* + * Check to make sure Stream has at least the 23 padding bytes in it + * prior to seeking. + */ + if (Stream_GetRemainingLength(input_stream) < 23) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "File stream does " + "not contain the expected number of bytes. File sharing " + "may not work as expected."); return; + } Stream_Seek(input_stream, 23); /* Padding */ diff --git a/src/protocols/rdp/channels/rdpdr/rdpdr-messages.c b/src/protocols/rdp/channels/rdpdr/rdpdr-messages.c index 76808fac..213da916 100644 --- a/src/protocols/rdp/channels/rdpdr/rdpdr-messages.c +++ b/src/protocols/rdp/channels/rdpdr/rdpdr-messages.c @@ -212,6 +212,7 @@ void guac_rdpdr_process_server_announce(guac_rdp_common_svc* svc, unsigned int major, minor, client_id; + /* Stream should contain at least 8 bytes (UINT16 + UINT16 + UINT32) */ if (Stream_GetRemainingLength(input_stream) < 8) return; @@ -246,8 +247,13 @@ void guac_rdpdr_process_device_reply(guac_rdp_common_svc* svc, unsigned int device_id, ntstatus; int severity, c, n, facility, code; - if (Stream_GetRemainingLength(input_stream) < 8) + /* Stream should contain at least 8 bytes (UINT32 + UINT32 ) */ + if (Stream_GetRemainingLength(input_stream) < 8) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Device Stream does not " + "contain the expected number of bytes. Device redirection may " + "not work."); return; + } Stream_Read_UINT32(input_stream, device_id); Stream_Read_UINT32(input_stream, ntstatus); @@ -284,8 +290,13 @@ void guac_rdpdr_process_device_iorequest(guac_rdp_common_svc* svc, guac_rdpdr* rdpdr = (guac_rdpdr*) svc->data; guac_rdpdr_iorequest iorequest; - if (Stream_GetRemainingLength(input_stream) < 20) + /* Check to make sure the Stream contains at least 20 bytes (5 x UINT32 ). */ + if (Stream_GetRemainingLength(input_stream) < 20) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Device Stream does not " + "contain the expected number of bytes. Device redirection may " + "not work as expected."); return; + } /* Read header */ Stream_Read_UINT32(input_stream, iorequest.device_id); @@ -315,8 +326,13 @@ void guac_rdpdr_process_server_capability(guac_rdp_common_svc* svc, int count; int i; - if (Stream_GetRemainingLength(input_stream) < 4) + /* Check to make sure the Stream has at least 4 bytes (UINT16 + 2) */ + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Redirection Stream " + "does not contain the expected number of bytes. Device " + "redirection may not work as expected."); return; + } /* Read header */ Stream_Read_UINT16(input_stream, count); @@ -328,14 +344,24 @@ void guac_rdpdr_process_server_capability(guac_rdp_common_svc* svc, int type; int length; - if (Stream_GetRemainingLength(input_stream) < 4) + /* Make sure Stream has at least 4 bytes (UINT16 + UINT16) */ + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Redirection Stream " + "does not contain the expected number of bytes. Device " + "redirection may not work as expected."); break; + } Stream_Read_UINT16(input_stream, type); Stream_Read_UINT16(input_stream, length); - if (Stream_GetRemainingLength(input_stream) < (length - 4)) + /* Make sure Stream has required length remaining for Seek below. */ + if (Stream_GetRemainingLength(input_stream) < (length - 4)) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Redirection Stream " + "does not contain the expected number of bytes. Device " + "redirection may not work as expected."); break; + } /* Ignore all for now */ guac_client_log(svc->client, GUAC_LOG_DEBUG, "Ignoring server capability set type=0x%04x, length=%i", type, length); diff --git a/src/protocols/rdp/channels/rdpdr/rdpdr-printer.c b/src/protocols/rdp/channels/rdpdr/rdpdr-printer.c index fb0b1c95..f810143c 100644 --- a/src/protocols/rdp/channels/rdpdr/rdpdr-printer.c +++ b/src/protocols/rdp/channels/rdpdr/rdpdr-printer.c @@ -67,8 +67,13 @@ void guac_rdpdr_process_print_job_write(guac_rdp_common_svc* svc, int length; int status; - if (Stream_GetRemainingLength(input_stream) < 32) + /* Verify that Stream contains at least 32 bytes (UINT32 + 8 + 20) */ + if (Stream_GetRemainingLength(input_stream) < 32) { + guac_client_log(client, GUAC_LOG_WARNING, "Printer Stream does not " + "contain the required number of bytes. Print redirection may " + "not work as expected."); return; + } /* Read buffer of print data */ Stream_Read_UINT32(input_stream, length); diff --git a/src/protocols/rdp/channels/rdpdr/rdpdr.c b/src/protocols/rdp/channels/rdpdr/rdpdr.c index 5499e717..5be2c813 100644 --- a/src/protocols/rdp/channels/rdpdr/rdpdr.c +++ b/src/protocols/rdp/channels/rdpdr/rdpdr.c @@ -38,8 +38,16 @@ void guac_rdpdr_process_receive(guac_rdp_common_svc* svc, int component; int packet_id; - if (Stream_GetRemainingLength(input_stream) < 4) + /* + * Check that device redirection stream contains at least 4 bytes + * (UINT16 + UINT16). + */ + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Device redirection " + "Stream does not contain the required number of bytes. Device " + "redirection may not function as expected."); return; + } /* Read header */ Stream_Read_UINT16(input_stream, component); diff --git a/src/protocols/rdp/channels/rdpsnd/rdpsnd-messages.c b/src/protocols/rdp/channels/rdpsnd/rdpsnd-messages.c index 9e33c0e7..cd1ed8ba 100644 --- a/src/protocols/rdp/channels/rdpsnd/rdpsnd-messages.c +++ b/src/protocols/rdp/channels/rdpsnd/rdpsnd-messages.c @@ -50,8 +50,13 @@ void guac_rdpsnd_formats_handler(guac_rdp_common_svc* svc, /* Reset own format count */ rdpsnd->format_count = 0; - if (Stream_GetRemainingLength(input_stream) < 20) + /* Check to make sure the stream has at least 20 bytes, which */ + if (Stream_GetRemainingLength(input_stream) < 20) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio Stream does not " + "contain the expected number of bytes. Sound may not work as " + "expected."); return; + } /* Format header */ Stream_Seek(input_stream, 14); @@ -99,8 +104,13 @@ void guac_rdpsnd_formats_handler(guac_rdp_common_svc* svc, /* Remember position in stream */ Stream_GetPointer(input_stream, format_start); - if (Stream_GetRemainingLength(input_stream) < 18) + /* Check to make sure Stream has at least 18 bytes. */ + if (Stream_GetRemainingLength(input_stream) < 18) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio Stream does " + "not contain the expected number of bytes. Sound may " + "not work as expected."); return; + } /* Read format */ Stream_Read_UINT16(input_stream, format_tag); @@ -113,8 +123,13 @@ void guac_rdpsnd_formats_handler(guac_rdp_common_svc* svc, /* Skip past extra data */ Stream_Read_UINT16(input_stream, body_size); - if (Stream_GetRemainingLength(input_stream) < body_size) + /* Check that Stream has at least body_size bytes remaining. */ + if (Stream_GetRemainingLength(input_stream) < body_size) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio Stream does " + "not contain the expected number of bytes. Sound may " + "not work as expected."); return; + } Stream_Seek(input_stream, body_size); @@ -215,8 +230,13 @@ void guac_rdpsnd_training_handler(guac_rdp_common_svc* svc, guac_rdpsnd* rdpsnd = (guac_rdpsnd*) svc->data; - if (Stream_GetRemainingLength(input_stream) < 4) + /* Check to make sure audio stream contains a minimum number of bytes. */ + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Audio Stream does not " + "contain the expected number of bytes. Sound may not work as " + "expected."); return; + } /* Read timestamp and data size */ Stream_Read_UINT16(input_stream, rdpsnd->server_timestamp); @@ -245,8 +265,13 @@ void guac_rdpsnd_wave_info_handler(guac_rdp_common_svc* svc, guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_audio_stream* audio = rdp_client->audio; - if (Stream_GetRemainingLength(input_stream) < 12) + /* Check to make sure audio stream contains a minimum number of bytes. */ + if (Stream_GetRemainingLength(input_stream) < 12) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Audio stream does not " + "contain the expected number of bytes. Sound may not work as " + "expected."); return; + } /* Read wave information */ Stream_Read_UINT16(input_stream, rdpsnd->server_timestamp); diff --git a/src/protocols/rdp/channels/rdpsnd/rdpsnd.c b/src/protocols/rdp/channels/rdpsnd/rdpsnd.c index 522ddc49..cc65a161 100644 --- a/src/protocols/rdp/channels/rdpsnd/rdpsnd.c +++ b/src/protocols/rdp/channels/rdpsnd/rdpsnd.c @@ -35,9 +35,13 @@ void guac_rdpsnd_process_receive(guac_rdp_common_svc* svc, guac_rdpsnd* rdpsnd = (guac_rdpsnd*) svc->data; guac_rdpsnd_pdu_header header; - /* Check that we at least have a header. */ - if (Stream_GetRemainingLength(input_stream) < 4) + /* Check that we at least the 4 byte header (UINT8 + UINT8 + UINT16) */ + if (Stream_GetRemainingLength(input_stream) < 4) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Audio Stream does not " + "contain the expected number of bytes. Sound may not work as " + "expected."); return; + } /* Read RDPSND PDU header */ Stream_Read_UINT8(input_stream, header.message_type); @@ -53,9 +57,13 @@ void guac_rdpsnd_process_receive(guac_rdp_common_svc* svc, return; } - /* Check body size */ - if (Stream_GetRemainingLength(input_stream) < header.body_size) + /* Check Stream size against body size */ + if (Stream_GetRemainingLength(input_stream) < header.body_size) { + guac_client_log(svc->client, GUAC_LOG_WARNING, "Audio Stream does not " + "contain the expected number of bytes. Sound may not work as " + "expected."); return; + } /* Dispatch message to standard handlers */ switch (header.message_type) { diff --git a/src/protocols/rdp/plugins/guacai/guacai-messages.c b/src/protocols/rdp/plugins/guacai/guacai-messages.c index 8641002c..1b15ef72 100644 --- a/src/protocols/rdp/plugins/guacai/guacai-messages.c +++ b/src/protocols/rdp/plugins/guacai/guacai-messages.c @@ -39,6 +39,7 @@ static void guac_rdp_ai_read_format(wStream* stream, guac_rdp_ai_format* format) { + /* Check that we have at least 18 bytes (5 x UINT16, 2 x UINT32) */ if (Stream_GetRemainingLength(stream) < 18) return; @@ -51,7 +52,7 @@ static void guac_rdp_ai_read_format(wStream* stream, Stream_Read_UINT16(stream, format->bps); /* wBitsPerSample */ Stream_Read_UINT16(stream, format->data_size); /* cbSize */ - /* Read arbitrary data block (if applicable) */ + /* Read arbitrary data block (if applicable) and data is available. */ if (format->data_size != 0 && Stream_GetRemainingLength(stream) >= format->data_size) { format->data = Stream_Pointer(stream); /* data */ @@ -236,9 +237,11 @@ static void guac_rdp_ai_send_formatchange(IWTSVirtualChannel* channel, void guac_rdp_ai_process_version(guac_client* client, IWTSVirtualChannel* channel, wStream* stream) { + /* Verify we have at least 4 bytes available (UINT32) */ if (Stream_GetRemainingLength(stream) < 4) { - guac_client_log(client, GUAC_LOG_WARNING, - "Invalid value provided for AUDIO_INPUT version."); + guac_client_log(client, GUAC_LOG_WARNING, "Audio input stream does not " + "contain the expected number of bytes. Audio input may not " + "work as expected."); return; } @@ -268,8 +271,13 @@ void guac_rdp_ai_process_formats(guac_client* client, guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_rdp_audio_buffer* audio_buffer = rdp_client->audio_input; - if (Stream_GetRemainingLength(stream) < 8) + /* Verify we have at least 8 bytes available (2 x UINT32) */ + if (Stream_GetRemainingLength(stream) < 8) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio input stream does not " + "contain the expected number of bytes. Audio input may not " + "work as expected."); return; + } UINT32 num_formats; Stream_Read_UINT32(stream, num_formats); /* NumFormats */ @@ -319,8 +327,13 @@ void guac_rdp_ai_process_open(guac_client* client, guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_rdp_audio_buffer* audio_buffer = rdp_client->audio_input; - if (Stream_GetRemainingLength(stream) < 8) + /* Verify we have at least 8 bytes available (2 x UINT32) */ + if (Stream_GetRemainingLength(stream) < 8) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio input stream does not " + "contain the expected number of bytes. Audio input may not " + "work as expected."); return; + } UINT32 packet_frames; UINT32 initial_format; diff --git a/src/protocols/rdp/plugins/guacai/guacai.c b/src/protocols/rdp/plugins/guacai/guacai.c index 5f2319d6..b0b0cb0d 100644 --- a/src/protocols/rdp/plugins/guacai/guacai.c +++ b/src/protocols/rdp/plugins/guacai/guacai.c @@ -52,8 +52,13 @@ static void guac_rdp_ai_handle_data(guac_client* client, IWTSVirtualChannel* channel, wStream* stream) { - if (Stream_GetRemainingLength(stream) < 1) + /* Verify we have at least 1 byte in the stream (UINT8) */ + if (Stream_GetRemainingLength(stream) < 1) { + guac_client_log(client, GUAC_LOG_WARNING, "Audio input stream does not " + "contain the expected number of bytes. Audio input may not " + "work as expected."); return; + } /* Read message ID from received PDU */ BYTE message_id;