GUACAMOLE-1059: Add explanatory comments and additional logging.

This commit is contained in:
Nick Couchman 2020-05-04 17:44:15 -04:00
parent 8560ff9718
commit 98f0c271fb
9 changed files with 162 additions and 32 deletions

View File

@ -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]; char destination_path[GUAC_RDP_FS_MAX_PATH];
/* Check stream size prior to reading. */ /* 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; return;
}
/* Read structure */ /* Read structure */
Stream_Seek_UINT8(input_stream); /* ReplaceIfExists */ Stream_Seek_UINT8(input_stream); /* ReplaceIfExists */
Stream_Seek_UINT8(input_stream); /* RootDirectory */ Stream_Seek_UINT8(input_stream); /* RootDirectory */

View File

@ -49,8 +49,12 @@ void guac_rdpdr_fs_process_create(guac_rdp_common_svc* svc,
char path[GUAC_RDP_FS_MAX_PATH]; char path[GUAC_RDP_FS_MAX_PATH];
/* Check remaining stream data prior to reading. */ /* 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; return;
}
/* Read "create" information */ /* Read "create" information */
Stream_Read_UINT32(input_stream, desired_access); 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; wStream* output_stream;
/* Check remaining bytes before reading 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; return;
}
/* Read packet */ /* Read packet */
Stream_Read_UINT32(input_stream, length); Stream_Read_UINT32(input_stream, length);
@ -181,8 +189,12 @@ void guac_rdpdr_fs_process_write(guac_rdp_common_svc* svc,
wStream* output_stream; wStream* output_stream;
/* Check remaining 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; return;
}
/* Read packet */ /* Read packet */
Stream_Read_UINT32(input_stream, length); 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; int fs_information_class;
/* Check remaining length */ /* 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; return;
}
Stream_Read_UINT32(input_stream, fs_information_class); 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; int fs_information_class;
/* Check remaining length */ /* 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; return;
}
Stream_Read_UINT32(input_stream, fs_information_class); 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; int length;
/* Check remaining 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; return;
}
Stream_Read_UINT32(input_stream, fs_information_class); Stream_Read_UINT32(input_stream, fs_information_class);
Stream_Read_UINT32(input_stream, length); /* Length */ 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) if (file == NULL)
return; 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; return;
}
/* Read main header */ /* Read main header */
Stream_Read_UINT32(input_stream, fs_information_class); 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 this is the first query, the path is included after padding */
if (initial_query) { 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; return;
}
Stream_Seek(input_stream, 23); /* Padding */ Stream_Seek(input_stream, 23); /* Padding */

View File

@ -212,6 +212,7 @@ void guac_rdpdr_process_server_announce(guac_rdp_common_svc* svc,
unsigned int major, minor, client_id; unsigned int major, minor, client_id;
/* Stream should contain at least 8 bytes (UINT16 + UINT16 + UINT32) */
if (Stream_GetRemainingLength(input_stream) < 8) if (Stream_GetRemainingLength(input_stream) < 8)
return; return;
@ -246,8 +247,13 @@ void guac_rdpdr_process_device_reply(guac_rdp_common_svc* svc,
unsigned int device_id, ntstatus; unsigned int device_id, ntstatus;
int severity, c, n, facility, code; 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; return;
}
Stream_Read_UINT32(input_stream, device_id); Stream_Read_UINT32(input_stream, device_id);
Stream_Read_UINT32(input_stream, ntstatus); 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* rdpdr = (guac_rdpdr*) svc->data;
guac_rdpdr_iorequest iorequest; 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; return;
}
/* Read header */ /* Read header */
Stream_Read_UINT32(input_stream, iorequest.device_id); 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 count;
int i; 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; return;
}
/* Read header */ /* Read header */
Stream_Read_UINT16(input_stream, count); Stream_Read_UINT16(input_stream, count);
@ -328,14 +344,24 @@ void guac_rdpdr_process_server_capability(guac_rdp_common_svc* svc,
int type; int type;
int length; 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; break;
}
Stream_Read_UINT16(input_stream, type); Stream_Read_UINT16(input_stream, type);
Stream_Read_UINT16(input_stream, length); 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; break;
}
/* Ignore all for now */ /* Ignore all for now */
guac_client_log(svc->client, GUAC_LOG_DEBUG, "Ignoring server capability set type=0x%04x, length=%i", type, length); guac_client_log(svc->client, GUAC_LOG_DEBUG, "Ignoring server capability set type=0x%04x, length=%i", type, length);

View File

@ -67,8 +67,13 @@ void guac_rdpdr_process_print_job_write(guac_rdp_common_svc* svc,
int length; int length;
int status; 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; return;
}
/* Read buffer of print data */ /* Read buffer of print data */
Stream_Read_UINT32(input_stream, length); Stream_Read_UINT32(input_stream, length);

View File

@ -38,8 +38,16 @@ void guac_rdpdr_process_receive(guac_rdp_common_svc* svc,
int component; int component;
int packet_id; 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; return;
}
/* Read header */ /* Read header */
Stream_Read_UINT16(input_stream, component); Stream_Read_UINT16(input_stream, component);

View File

@ -50,8 +50,13 @@ void guac_rdpsnd_formats_handler(guac_rdp_common_svc* svc,
/* Reset own format count */ /* Reset own format count */
rdpsnd->format_count = 0; 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; return;
}
/* Format header */ /* Format header */
Stream_Seek(input_stream, 14); Stream_Seek(input_stream, 14);
@ -99,8 +104,13 @@ void guac_rdpsnd_formats_handler(guac_rdp_common_svc* svc,
/* Remember position in stream */ /* Remember position in stream */
Stream_GetPointer(input_stream, format_start); 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; return;
}
/* Read format */ /* Read format */
Stream_Read_UINT16(input_stream, format_tag); 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 */ /* Skip past extra data */
Stream_Read_UINT16(input_stream, body_size); 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; return;
}
Stream_Seek(input_stream, body_size); 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; 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; return;
}
/* Read timestamp and data size */ /* Read timestamp and data size */
Stream_Read_UINT16(input_stream, rdpsnd->server_timestamp); 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_rdp_client* rdp_client = (guac_rdp_client*) client->data;
guac_audio_stream* audio = rdp_client->audio; 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; return;
}
/* Read wave information */ /* Read wave information */
Stream_Read_UINT16(input_stream, rdpsnd->server_timestamp); Stream_Read_UINT16(input_stream, rdpsnd->server_timestamp);

View File

@ -35,9 +35,13 @@ void guac_rdpsnd_process_receive(guac_rdp_common_svc* svc,
guac_rdpsnd* rdpsnd = (guac_rdpsnd*) svc->data; guac_rdpsnd* rdpsnd = (guac_rdpsnd*) svc->data;
guac_rdpsnd_pdu_header header; guac_rdpsnd_pdu_header header;
/* Check that we at least have a header. */ /* Check that we at least the 4 byte header (UINT8 + UINT8 + UINT16) */
if (Stream_GetRemainingLength(input_stream) < 4) 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; return;
}
/* Read RDPSND PDU header */ /* Read RDPSND PDU header */
Stream_Read_UINT8(input_stream, header.message_type); Stream_Read_UINT8(input_stream, header.message_type);
@ -53,9 +57,13 @@ void guac_rdpsnd_process_receive(guac_rdp_common_svc* svc,
return; return;
} }
/* Check body size */ /* Check Stream size against body size */
if (Stream_GetRemainingLength(input_stream) < header.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; return;
}
/* Dispatch message to standard handlers */ /* Dispatch message to standard handlers */
switch (header.message_type) { switch (header.message_type) {

View File

@ -39,6 +39,7 @@
static void guac_rdp_ai_read_format(wStream* stream, static void guac_rdp_ai_read_format(wStream* stream,
guac_rdp_ai_format* format) { guac_rdp_ai_format* format) {
/* Check that we have at least 18 bytes (5 x UINT16, 2 x UINT32) */
if (Stream_GetRemainingLength(stream) < 18) if (Stream_GetRemainingLength(stream) < 18)
return; 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->bps); /* wBitsPerSample */
Stream_Read_UINT16(stream, format->data_size); /* cbSize */ 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 if (format->data_size != 0
&& Stream_GetRemainingLength(stream) >= format->data_size) { && Stream_GetRemainingLength(stream) >= format->data_size) {
format->data = Stream_Pointer(stream); /* data */ 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, void guac_rdp_ai_process_version(guac_client* client,
IWTSVirtualChannel* channel, wStream* stream) { IWTSVirtualChannel* channel, wStream* stream) {
/* Verify we have at least 4 bytes available (UINT32) */
if (Stream_GetRemainingLength(stream) < 4) { if (Stream_GetRemainingLength(stream) < 4) {
guac_client_log(client, GUAC_LOG_WARNING, guac_client_log(client, GUAC_LOG_WARNING, "Audio input stream does not "
"Invalid value provided for AUDIO_INPUT version."); "contain the expected number of bytes. Audio input may not "
"work as expected.");
return; 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_client* rdp_client = (guac_rdp_client*) client->data;
guac_rdp_audio_buffer* audio_buffer = rdp_client->audio_input; 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; return;
}
UINT32 num_formats; UINT32 num_formats;
Stream_Read_UINT32(stream, num_formats); /* NumFormats */ 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_client* rdp_client = (guac_rdp_client*) client->data;
guac_rdp_audio_buffer* audio_buffer = rdp_client->audio_input; 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; return;
}
UINT32 packet_frames; UINT32 packet_frames;
UINT32 initial_format; UINT32 initial_format;

View File

@ -52,8 +52,13 @@
static void guac_rdp_ai_handle_data(guac_client* client, static void guac_rdp_ai_handle_data(guac_client* client,
IWTSVirtualChannel* channel, wStream* stream) { 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; return;
}
/* Read message ID from received PDU */ /* Read message ID from received PDU */
BYTE message_id; BYTE message_id;