From deffd0d781f90e4a44b593450d5f9de6f99c839d Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Sun, 24 Jan 2016 15:21:06 +0100 Subject: [PATCH 1/7] Fixed argument checks for drive channel. --- channels/rdpdr/client/devman.c | 22 ++++++ channels/rdpdr/client/rdpdr_capabilities.c | 7 +- channels/rdpdr/client/rdpdr_capabilities.h | 2 +- channels/rdpdr/client/rdpdr_main.c | 78 ++++++++++++++++++---- 4 files changed, 95 insertions(+), 14 deletions(-) diff --git a/channels/rdpdr/client/devman.c b/channels/rdpdr/client/devman.c index 2e1666204..315339d92 100644 --- a/channels/rdpdr/client/devman.c +++ b/channels/rdpdr/client/devman.c @@ -6,6 +6,7 @@ * Copyright 2010-2012 Marc-Andre Moreau * Copyright 2015 Thincast Technologies GmbH * Copyright 2015 DI (FH) Martin Haimberger + * Copyright 2016 Armin Novak * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +43,9 @@ void devman_device_free(DEVICE* device) { + if (!device) + return; + IFCALL(device->Free, device); } @@ -49,6 +53,9 @@ DEVMAN* devman_new(rdpdrPlugin* rdpdr) { DEVMAN* devman; + if (!rdpdr) + return NULL; + devman = (DEVMAN*) calloc(1, sizeof(DEVMAN)); if (!devman) @@ -84,6 +91,9 @@ void devman_unregister_device(DEVMAN* devman, void* key) { DEVICE* device; + if (!devman || !key) + return; + device = (DEVICE*) ListDictionary_Remove(devman->devices, key); if (device) @@ -99,6 +109,12 @@ static UINT devman_register_device(DEVMAN* devman, DEVICE* device) { void* key = NULL; + if (!devman || !device) + return ERROR_INVALID_PARAMETER; + + if (device->type != RDPDR_DTYP_FILESYSTEM) + return CHANNEL_RC_OK; + device->id = devman->id_sequence++; key = (void*) (size_t) device->id; @@ -115,6 +131,9 @@ DEVICE* devman_get_device_by_id(DEVMAN* devman, UINT32 id) DEVICE* device = NULL; void* key = (void*) (size_t) id; + if (!devman) + return NULL; + device = (DEVICE*) ListDictionary_GetItemValue(devman->devices, key); return device; @@ -137,6 +156,9 @@ UINT devman_load_device_service(DEVMAN* devman, RDPDR_DEVICE* device, rdpContext DEVICE_SERVICE_ENTRY_POINTS ep; PDEVICE_SERVICE_ENTRY entry = NULL; + if (!devman || !device || !rdpcontext) + return ERROR_INVALID_PARAMETER; + if (device->Type == RDPDR_DTYP_FILESYSTEM) ServiceName = DRIVE_SERVICE_NAME; else if (device->Type == RDPDR_DTYP_PRINT) diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index 26f6d529e..1198a53c9 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -128,12 +128,15 @@ static void rdpdr_process_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) Stream_Seek(s, capabilityLength - 4); } -void rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) +UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) { UINT16 i; UINT16 numCapabilities; UINT16 capabilityType; + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + Stream_Read_UINT16(s, numCapabilities); Stream_Seek(s, 2); /* pad (2 bytes) */ @@ -167,6 +170,8 @@ void rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) break; } } + + return CHANNEL_RC_OK; } /** diff --git a/channels/rdpdr/client/rdpdr_capabilities.h b/channels/rdpdr/client/rdpdr_capabilities.h index bc2ef8b9e..d4e1ecb27 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.h +++ b/channels/rdpdr/client/rdpdr_capabilities.h @@ -25,7 +25,7 @@ #include "rdpdr_main.h" -void rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s); +UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s); UINT rdpdr_send_capability_response(rdpdrPlugin* rdpdr); #endif /* FREERDP_CHANNEL_RDPDR_CLIENT_CAPABILITIES_H */ diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 30bd2681d..9581ab0ae 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -81,6 +81,9 @@ static UINT rdpdr_send_device_list_remove_request(rdpdrPlugin* rdpdr, UINT32 cou UINT32 i; wStream* s; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + s = Stream_New(NULL, 256); if (!s) { @@ -429,7 +432,7 @@ static UINT handle_hotplug(rdpdrPlugin* rdpdr) BOOL dev_found = FALSE; device_ext = (DEVICE_DRIVE_EXT *)ListDictionary_GetItemValue(rdpdr->devman->devices, (void *)keys[j]); - if (!device_ext) + if (!device_ext || !device_ext->path) continue; /* not plugable device */ @@ -592,6 +595,10 @@ out: static UINT drive_hotplug_thread_terminate(rdpdrPlugin* rdpdr) { UINT error; + + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + if (rdpdr->hotplugThread) { if (rdpdr->stopEvent) @@ -623,6 +630,9 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) rdpSettings* settings; UINT error = CHANNEL_RC_OK; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + rdpdr->devman = devman_new(rdpdr); if (!rdpdr->devman) { @@ -661,13 +671,18 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) return error; } -static void rdpdr_process_server_announce_request(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_server_announce_request(rdpdrPlugin* rdpdr, wStream* s) { + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + Stream_Read_UINT16(s, rdpdr->versionMajor); Stream_Read_UINT16(s, rdpdr->versionMinor); Stream_Read_UINT32(s, rdpdr->clientID); rdpdr->sequenceId++; + + return CHANNEL_RC_OK; } /** @@ -679,6 +694,9 @@ static UINT rdpdr_send_client_announce_reply(rdpdrPlugin* rdpdr) { wStream* s; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + s = Stream_New(NULL, 12); if (!s) { @@ -707,6 +725,9 @@ static UINT rdpdr_send_client_name_request(rdpdrPlugin* rdpdr) WCHAR* computerNameW = NULL; size_t computerNameLenW; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + if (!rdpdr->computerName[0]) gethostname(rdpdr->computerName, sizeof(rdpdr->computerName) - 1); @@ -733,12 +754,15 @@ static UINT rdpdr_send_client_name_request(rdpdrPlugin* rdpdr) return rdpdr_send(rdpdr, s); } -static void rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s) { UINT16 versionMajor; UINT16 versionMinor; UINT32 clientID; + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + Stream_Read_UINT16(s, versionMajor); Stream_Read_UINT16(s, versionMinor); Stream_Read_UINT32(s, clientID); @@ -750,9 +774,9 @@ static void rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s } if (clientID != rdpdr->clientID) - { rdpdr->clientID = clientID; - } + + return CHANNEL_RC_OK; } /** @@ -774,6 +798,9 @@ static UINT rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use int keyCount; ULONG_PTR* pKeys; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + s = Stream_New(NULL, 256); if (!s) { @@ -859,6 +886,9 @@ static UINT rdpdr_process_irp(rdpdrPlugin* rdpdr, wStream* s) IRP* irp; UINT error = CHANNEL_RC_OK; + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + irp = irp_new(rdpdr->devman, s); if (!irp) @@ -888,6 +918,9 @@ static UINT rdpdr_process_init(rdpdrPlugin* rdpdr) ULONG_PTR* pKeys; UINT error = CHANNEL_RC_OK; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + pKeys = NULL; keyCount = ListDictionary_GetKeys(rdpdr->devman->devices, &pKeys); @@ -921,6 +954,9 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) UINT32 status; UINT error; + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + Stream_Read_UINT16(s, component); /* Component (2 bytes) */ Stream_Read_UINT16(s, packetId); /* PacketId (2 bytes) */ @@ -929,7 +965,8 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) switch (packetId) { case PAKID_CORE_SERVER_ANNOUNCE: - rdpdr_process_server_announce_request(rdpdr, s); + if ((error = rdpdr_process_server_announce_request(rdpdr, s))) + return error; if ((error = rdpdr_send_client_announce_reply(rdpdr))) { WLog_ERR(TAG, "rdpdr_send_client_announce_reply failed with error %lu", error); @@ -948,7 +985,8 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) break; case PAKID_CORE_SERVER_CAPABILITY: - rdpdr_process_capability_request(rdpdr, s); + if ((error = rdpdr_process_capability_request(rdpdr, s))) + return error; if ((error = rdpdr_send_capability_response(rdpdr))) { WLog_ERR(TAG, "rdpdr_send_capability_response failed with error %lu", error); @@ -957,7 +995,9 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) break; case PAKID_CORE_CLIENTID_CONFIRM: - rdpdr_process_server_clientid_confirm(rdpdr, s); + if ((error = rdpdr_process_server_clientid_confirm(rdpdr, s))) + return error; + if ((error = rdpdr_send_device_list_announce_request(rdpdr, FALSE))) { WLog_ERR(TAG, "rdpdr_send_device_list_announce_request failed with error %lu", error); @@ -1132,10 +1172,11 @@ UINT rdpdr_send(rdpdrPlugin* rdpdr, wStream* s) UINT status; rdpdrPlugin* plugin = (rdpdrPlugin*) rdpdr; + if (!rdpdr || !s) + return ERROR_INVALID_PARAMETER; + if (!plugin) - { status = CHANNEL_RC_BAD_INIT_HANDLE; - } else { status = plugin->channelEntryPoints.pVirtualChannelWrite(plugin->OpenHandle, @@ -1162,6 +1203,9 @@ static UINT rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, { wStream* data_in; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + if ((dataFlags & CHANNEL_FLAG_SUSPEND) || (dataFlags & CHANNEL_FLAG_RESUME)) { /* @@ -1256,6 +1300,12 @@ static void* rdpdr_virtual_channel_client_thread(void* arg) rdpdrPlugin* rdpdr = (rdpdrPlugin*) arg; UINT error; + if (!rdpdr) + { + ExitThread((DWORD) ERROR_INVALID_PARAMETER); + return NULL; + } + if ((error = rdpdr_process_connect(rdpdr))) { WLog_ERR(TAG, "rdpdr_process_connect failed with error %lu!", error); @@ -1304,6 +1354,9 @@ static UINT rdpdr_virtual_channel_event_connected(rdpdrPlugin* rdpdr, LPVOID pDa UINT32 status; UINT error; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + status = rdpdr->channelEntryPoints.pVirtualChannelOpen(rdpdr->InitHandle, &rdpdr->OpenHandle, rdpdr->channelDef.name, rdpdr_virtual_channel_open_event); @@ -1345,6 +1398,9 @@ static UINT rdpdr_virtual_channel_event_disconnected(rdpdrPlugin* rdpdr) { UINT error; + if (!rdpdr) + return ERROR_INVALID_PARAMETER; + if (MessageQueue_PostQuit(rdpdr->queue, 0) && (WaitForSingleObject(rdpdr->thread, INFINITE) == WAIT_FAILED)) { error = GetLastError(); @@ -1444,7 +1500,6 @@ BOOL VCAPITYPE VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints) rdpdrPlugin* rdpdr; CHANNEL_ENTRY_POINTS_FREERDP* pEntryPointsEx; - rdpdr = (rdpdrPlugin*) calloc(1, sizeof(rdpdrPlugin)); if (!rdpdr) @@ -1473,7 +1528,6 @@ BOOL VCAPITYPE VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints) CopyMemory(&(rdpdr->channelEntryPoints), pEntryPoints, sizeof(CHANNEL_ENTRY_POINTS_FREERDP)); - rc = rdpdr->channelEntryPoints.pVirtualChannelInit(&rdpdr->InitHandle, &rdpdr->channelDef, 1, VIRTUAL_CHANNEL_VERSION_WIN2000, rdpdr_virtual_channel_init_event); From e08ca73ddcf2a4d8dcbb747c92629b54547eb687 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 27 Jan 2016 19:26:52 +0100 Subject: [PATCH 2/7] Improved error checks. --- channels/rdpdr/client/rdpdr_capabilities.c | 101 ++++-- channels/rdpdr/client/rdpdr_main.c | 360 ++++++++++----------- 2 files changed, 246 insertions(+), 215 deletions(-) diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index 1198a53c9..4de411674 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -60,12 +60,21 @@ static void rdpdr_write_general_capset(rdpdrPlugin* rdpdr, wStream* s) } /* Process device direction general capability set */ -static void rdpdr_process_general_capset(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_general_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; + if (!Stream_EnsureRemainingCapacity(s, 2)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityLength); + + if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + return CHANNEL_RC_NO_BUFFER; + Stream_Seek(s, capabilityLength - 4); + + return CHANNEL_RC_OK; } /* Output printer direction capability set */ @@ -75,12 +84,21 @@ static void rdpdr_write_printer_capset(rdpdrPlugin* rdpdr, wStream* s) } /* Process printer direction capability set */ -static void rdpdr_process_printer_capset(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_printer_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; + if (!Stream_EnsureRemainingCapacity(s, 2)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityLength); + + if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + return CHANNEL_RC_NO_BUFFER; + Stream_Seek(s, capabilityLength - 4); + + return CHANNEL_RC_OK; } /* Output port redirection capability set */ @@ -90,12 +108,21 @@ static void rdpdr_write_port_capset(rdpdrPlugin* rdpdr, wStream* s) } /* Process port redirection capability set */ -static void rdpdr_process_port_capset(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_port_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; + if (!Stream_EnsureRemainingCapacity(s, 2)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityLength); + + if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + return CHANNEL_RC_NO_BUFFER; + Stream_Seek(s, capabilityLength - 4); + + return CHANNEL_RC_OK; } /* Output drive redirection capability set */ @@ -105,12 +132,21 @@ static void rdpdr_write_drive_capset(rdpdrPlugin* rdpdr, wStream* s) } /* Process drive redirection capability set */ -static void rdpdr_process_drive_capset(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_drive_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; + if (!Stream_EnsureRemainingCapacity(s, 2)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityLength); + + if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + return CHANNEL_RC_NO_BUFFER; + Stream_Seek(s, capabilityLength - 4); + + return CHANNEL_RC_OK; } /* Output smart card redirection capability set */ @@ -120,55 +156,74 @@ static void rdpdr_write_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) } /* Process smartcard redirection capability set */ -static void rdpdr_process_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) +static UINT rdpdr_process_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; + if (!Stream_EnsureRemainingCapacity(s, 2)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityLength); + + if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + return CHANNEL_RC_NO_BUFFER; + Stream_Seek(s, capabilityLength - 4); + + return CHANNEL_RC_OK; } UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) { + UINT status = CHANNEL_RC_OK; UINT16 i; UINT16 numCapabilities; UINT16 capabilityType; if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; + return CHANNEL_RC_NULL_DATA; + + if (!Stream_EnsureRemainingCapacity(s, 4)) + return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, numCapabilities); Stream_Seek(s, 2); /* pad (2 bytes) */ + if (!Stream_EnsureRemainingCapacity(s, sizeof(UINT16) * numCapabilities)) + return CHANNEL_RC_NO_BUFFER; + for (i = 0; i < numCapabilities; i++) { Stream_Read_UINT16(s, capabilityType); switch (capabilityType) { - case CAP_GENERAL_TYPE: - rdpdr_process_general_capset(rdpdr, s); - break; + case CAP_GENERAL_TYPE: + status = rdpdr_process_general_capset(rdpdr, s); + break; - case CAP_PRINTER_TYPE: - rdpdr_process_printer_capset(rdpdr, s); - break; + case CAP_PRINTER_TYPE: + rdpdr_process_printer_capset(rdpdr, s); + break; - case CAP_PORT_TYPE: - rdpdr_process_port_capset(rdpdr, s); - break; + case CAP_PORT_TYPE: + rdpdr_process_port_capset(rdpdr, s); + break; - case CAP_DRIVE_TYPE: - rdpdr_process_drive_capset(rdpdr, s); - break; + case CAP_DRIVE_TYPE: + rdpdr_process_drive_capset(rdpdr, s); + break; - case CAP_SMARTCARD_TYPE: - rdpdr_process_smartcard_capset(rdpdr, s); - break; + case CAP_SMARTCARD_TYPE: + rdpdr_process_smartcard_capset(rdpdr, s); + break; - default: - break; + default: + break; } + + if (status != CHANNEL_RC_OK) + return status; } return CHANNEL_RC_OK; diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 9581ab0ae..ae097fa87 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -81,10 +81,7 @@ static UINT rdpdr_send_device_list_remove_request(rdpdrPlugin* rdpdr, UINT32 cou UINT32 i; wStream* s; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - - s = Stream_New(NULL, 256); + s = Stream_New(NULL, count * sizeof(UINT32) + 8); if (!s) { WLog_ERR(TAG, "Stream_New failed!"); @@ -520,15 +517,15 @@ static void* drive_hotplug_thread_func(void* arg) struct timeval tv; int rv; UINT error; - DWORD status; + DWORD status; rdpdr = (rdpdrPlugin*) arg; if (!(rdpdr->stopEvent = CreateEvent(NULL, TRUE, FALSE, NULL))) { WLog_ERR(TAG, "CreateEvent failed!"); - error = ERROR_INTERNAL_ERROR; - goto out; + error = ERROR_INTERNAL_ERROR; + goto out; } mfd = open("/proc/mounts", O_RDONLY, 0); @@ -536,8 +533,8 @@ static void* drive_hotplug_thread_func(void* arg) if (mfd < 0) { WLog_ERR(TAG, "ERROR: Unable to open /proc/mounts."); - error = ERROR_INTERNAL_ERROR; - goto out; + error = ERROR_INTERNAL_ERROR; + goto out; } FD_ZERO(&rfds); @@ -548,18 +545,18 @@ static void* drive_hotplug_thread_func(void* arg) if ((error = handle_hotplug(rdpdr))) { WLog_ERR(TAG, "handle_hotplug failed with error %lu!", error); - goto out; + goto out; } while ((rv = select(mfd+1, NULL, NULL, &rfds, &tv)) >= 0) { - status = WaitForSingleObject(rdpdr->stopEvent, 0); - if (status == WAIT_FAILED) - { - error = GetLastError(); - WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); - goto out; - } + status = WaitForSingleObject(rdpdr->stopEvent, 0); + if (status == WAIT_FAILED) + { + error = GetLastError(); + WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); + goto out; + } if (status == WAIT_OBJECT_0) break; @@ -569,7 +566,7 @@ static void* drive_hotplug_thread_func(void* arg) if ((error = handle_hotplug(rdpdr))) { WLog_ERR(TAG, "handle_hotplug failed with error %lu!", error); - goto out; + goto out; } } @@ -580,11 +577,11 @@ static void* drive_hotplug_thread_func(void* arg) } out: - if (error && rdpdr->rdpcontext) - setChannelError(rdpdr->rdpcontext, error, "drive_hotplug_thread_func reported an error"); + if (error && rdpdr->rdpcontext) + setChannelError(rdpdr->rdpcontext, error, "drive_hotplug_thread_func reported an error"); - ExitThread((DWORD)error); - return NULL; + ExitThread((DWORD)error); + return NULL; } /** @@ -594,10 +591,7 @@ out: */ static UINT drive_hotplug_thread_terminate(rdpdrPlugin* rdpdr) { - UINT error; - - if (!rdpdr) - return ERROR_INVALID_PARAMETER; + UINT error; if (rdpdr->hotplugThread) { @@ -605,14 +599,14 @@ static UINT drive_hotplug_thread_terminate(rdpdrPlugin* rdpdr) SetEvent(rdpdr->stopEvent); if (WaitForSingleObject(rdpdr->hotplugThread, INFINITE) == WAIT_FAILED) - { - error = GetLastError(); - WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); - return error; - } + { + error = GetLastError(); + WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); + return error; + } rdpdr->hotplugThread = NULL; } - return CHANNEL_RC_OK; + return CHANNEL_RC_OK; } #endif @@ -630,9 +624,6 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) rdpSettings* settings; UINT error = CHANNEL_RC_OK; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - rdpdr->devman = devman_new(rdpdr); if (!rdpdr->devman) { @@ -673,8 +664,8 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) static UINT rdpdr_process_server_announce_request(rdpdrPlugin* rdpdr, wStream* s) { - if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; + if (!Stream_EnsureRemainingCapacity(s, 8)) + return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, rdpdr->versionMajor); Stream_Read_UINT16(s, rdpdr->versionMinor); @@ -694,14 +685,11 @@ static UINT rdpdr_send_client_announce_reply(rdpdrPlugin* rdpdr) { wStream* s; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - s = Stream_New(NULL, 12); if (!s) { WLog_ERR(TAG, "Stream_New failed!"); - return CHANNEL_RC_OK; + return CHANNEL_RC_NO_MEMORY; } Stream_Write_UINT16(s, RDPDR_CTYP_CORE); /* Component (2 bytes) */ @@ -725,9 +713,6 @@ static UINT rdpdr_send_client_name_request(rdpdrPlugin* rdpdr) WCHAR* computerNameW = NULL; size_t computerNameLenW; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - if (!rdpdr->computerName[0]) gethostname(rdpdr->computerName, sizeof(rdpdr->computerName) - 1); @@ -760,8 +745,8 @@ static UINT rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s UINT16 versionMinor; UINT32 clientID; - if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; + if (!Stream_EnsureRemainingCapacity(s, 8)) + return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, versionMajor); Stream_Read_UINT16(s, versionMinor); @@ -798,9 +783,6 @@ static UINT rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use int keyCount; ULONG_PTR* pKeys; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - s = Stream_New(NULL, 256); if (!s) { @@ -831,7 +813,7 @@ static UINT rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use */ if ((rdpdr->versionMinor == 0x0005) || - (device->type == RDPDR_DTYP_SMARTCARD) || userLoggedOn) + (device->type == RDPDR_DTYP_SMARTCARD) || userLoggedOn) { data_len = (int) (device->data == NULL ? 0 : Stream_GetPosition(device->data)); if (!Stream_EnsureRemainingCapacity(s, 20 + data_len)) @@ -886,9 +868,6 @@ static UINT rdpdr_process_irp(rdpdrPlugin* rdpdr, wStream* s) IRP* irp; UINT error = CHANNEL_RC_OK; - if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; - irp = irp_new(rdpdr->devman, s); if (!irp) @@ -899,10 +878,10 @@ static UINT rdpdr_process_irp(rdpdrPlugin* rdpdr, wStream* s) IFCALLRET(irp->device->IRPRequest, error, irp->device, irp); - if (error) - WLog_ERR(TAG, "device->IRPRequest failed with error %lu", error); + if (error) + WLog_ERR(TAG, "device->IRPRequest failed with error %lu", error); - return error; + return error; } /** @@ -918,9 +897,6 @@ static UINT rdpdr_process_init(rdpdrPlugin* rdpdr) ULONG_PTR* pKeys; UINT error = CHANNEL_RC_OK; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - pKeys = NULL; keyCount = ListDictionary_GetKeys(rdpdr->devman->devices, &pKeys); @@ -955,7 +931,10 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) UINT error; if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; + return CHANNEL_RC_NULL_DATA; + + if (!Stream_EnsureRemainingCapacity(s, 4)) + return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, component); /* Component (2 bytes) */ Stream_Read_UINT16(s, packetId); /* PacketId (2 bytes) */ @@ -964,74 +943,77 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) { switch (packetId) { - case PAKID_CORE_SERVER_ANNOUNCE: - if ((error = rdpdr_process_server_announce_request(rdpdr, s))) - return error; - if ((error = rdpdr_send_client_announce_reply(rdpdr))) - { - WLog_ERR(TAG, "rdpdr_send_client_announce_reply failed with error %lu", error); - return error; - } - if ((error = rdpdr_send_client_name_request(rdpdr))) - { - WLog_ERR(TAG, "rdpdr_send_client_name_request failed with error %lu", error); - return error; - } - if ((error = rdpdr_process_init(rdpdr))) - { - WLog_ERR(TAG, "rdpdr_process_init failed with error %lu", error); - return error; - } - break; + case PAKID_CORE_SERVER_ANNOUNCE: + if ((error = rdpdr_process_server_announce_request(rdpdr, s))) + return error; + if ((error = rdpdr_send_client_announce_reply(rdpdr))) + { + WLog_ERR(TAG, "rdpdr_send_client_announce_reply failed with error %lu", error); + return error; + } + if ((error = rdpdr_send_client_name_request(rdpdr))) + { + WLog_ERR(TAG, "rdpdr_send_client_name_request failed with error %lu", error); + return error; + } + if ((error = rdpdr_process_init(rdpdr))) + { + WLog_ERR(TAG, "rdpdr_process_init failed with error %lu", error); + return error; + } + break; - case PAKID_CORE_SERVER_CAPABILITY: - if ((error = rdpdr_process_capability_request(rdpdr, s))) - return error; - if ((error = rdpdr_send_capability_response(rdpdr))) - { - WLog_ERR(TAG, "rdpdr_send_capability_response failed with error %lu", error); - return error; - } - break; + case PAKID_CORE_SERVER_CAPABILITY: + if ((error = rdpdr_process_capability_request(rdpdr, s))) + return error; + if ((error = rdpdr_send_capability_response(rdpdr))) + { + WLog_ERR(TAG, "rdpdr_send_capability_response failed with error %lu", error); + return error; + } + break; - case PAKID_CORE_CLIENTID_CONFIRM: - if ((error = rdpdr_process_server_clientid_confirm(rdpdr, s))) - return error; + case PAKID_CORE_CLIENTID_CONFIRM: + if ((error = rdpdr_process_server_clientid_confirm(rdpdr, s))) + return error; - if ((error = rdpdr_send_device_list_announce_request(rdpdr, FALSE))) - { - WLog_ERR(TAG, "rdpdr_send_device_list_announce_request failed with error %lu", error); - return error; - } - break; + if ((error = rdpdr_send_device_list_announce_request(rdpdr, FALSE))) + { + WLog_ERR(TAG, "rdpdr_send_device_list_announce_request failed with error %lu", error); + return error; + } + break; - case PAKID_CORE_USER_LOGGEDON: - if ((error = rdpdr_send_device_list_announce_request(rdpdr, TRUE))) - { - WLog_ERR(TAG, "rdpdr_send_device_list_announce_request failed with error %lu", error); - return error; - } - break; + case PAKID_CORE_USER_LOGGEDON: + if ((error = rdpdr_send_device_list_announce_request(rdpdr, TRUE))) + { + WLog_ERR(TAG, "rdpdr_send_device_list_announce_request failed with error %lu", error); + return error; + } + break; - case PAKID_CORE_DEVICE_REPLY: - /* connect to a specific resource */ - Stream_Read_UINT32(s, deviceId); - Stream_Read_UINT32(s, status); - break; + case PAKID_CORE_DEVICE_REPLY: + /* connect to a specific resource */ + if (Stream_EnsureRemainingCapacity(s, 8)) + return CHANNEL_RC_NO_BUFFER; - case PAKID_CORE_DEVICE_IOREQUEST: - if ((error = rdpdr_process_irp(rdpdr, s))) - { - WLog_ERR(TAG, "rdpdr_process_irp failed with error %lu", error); - return error; - } - s = NULL; - break; + Stream_Read_UINT32(s, deviceId); + Stream_Read_UINT32(s, status); + break; - default: - WLog_ERR(TAG, "RDPDR_CTYP_CORE unknown PacketId: 0x%04X", packetId); - return ERROR_INVALID_DATA; - break; + case PAKID_CORE_DEVICE_IOREQUEST: + if ((error = rdpdr_process_irp(rdpdr, s))) + { + WLog_ERR(TAG, "rdpdr_process_irp failed with error %lu", error); + return error; + } + s = NULL; + break; + + default: + WLog_ERR(TAG, "RDPDR_CTYP_CORE unknown PacketId: 0x%04X", packetId); + return ERROR_INVALID_DATA; + break; } } @@ -1039,21 +1021,24 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) { switch (packetId) { - case PAKID_PRN_CACHE_DATA: - { - UINT32 eventID; - Stream_Read_UINT32(s, eventID); - WLog_ERR(TAG, "Ignoring unhandled message PAKID_PRN_CACHE_DATA (EventID: 0x%04X)", eventID); - } - break; + case PAKID_PRN_CACHE_DATA: + { + UINT32 eventID; + if (Stream_EnsureRemainingCapacity(s, 4)) + return CHANNEL_RC_NO_BUFFER; - case PAKID_PRN_USING_XPS: - WLog_ERR(TAG, "Ignoring unhandled message PAKID_PRN_USING_XPS"); - break; + Stream_Read_UINT32(s, eventID); + WLog_ERR(TAG, "Ignoring unhandled message PAKID_PRN_CACHE_DATA (EventID: 0x%04X)", eventID); + } + break; - default: - WLog_ERR(TAG, "Unknown printing component packetID: 0x%04X", packetId); - return ERROR_INVALID_DATA; + case PAKID_PRN_USING_XPS: + WLog_ERR(TAG, "Ignoring unhandled message PAKID_PRN_USING_XPS"); + break; + + default: + WLog_ERR(TAG, "Unknown printing component packetID: 0x%04X", packetId); + return ERROR_INVALID_DATA; } } else @@ -1173,14 +1158,14 @@ UINT rdpdr_send(rdpdrPlugin* rdpdr, wStream* s) rdpdrPlugin* plugin = (rdpdrPlugin*) rdpdr; if (!rdpdr || !s) - return ERROR_INVALID_PARAMETER; + return CHANNEL_RC_NULL_DATA; if (!plugin) status = CHANNEL_RC_BAD_INIT_HANDLE; else { status = plugin->channelEntryPoints.pVirtualChannelWrite(plugin->OpenHandle, - Stream_Buffer(s), (UINT32) Stream_GetPosition(s), s); + Stream_Buffer(s), (UINT32) Stream_GetPosition(s), s); } if (status != CHANNEL_RC_OK) @@ -1199,13 +1184,10 @@ UINT rdpdr_send(rdpdrPlugin* rdpdr, wStream* s) * @return 0 on success, otherwise a Win32 error code */ static UINT rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, - void* pData, UINT32 dataLength, UINT32 totalLength, UINT32 dataFlags) + void* pData, UINT32 dataLength, UINT32 totalLength, UINT32 dataFlags) { wStream* data_in; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - if ((dataFlags & CHANNEL_FLAG_SUSPEND) || (dataFlags & CHANNEL_FLAG_RESUME)) { /* @@ -1260,14 +1242,14 @@ static UINT rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, } static VOID VCAPITYPE rdpdr_virtual_channel_open_event(DWORD openHandle, UINT event, - LPVOID pData, UINT32 dataLength, UINT32 totalLength, UINT32 dataFlags) + LPVOID pData, UINT32 dataLength, UINT32 totalLength, UINT32 dataFlags) { rdpdrPlugin* rdpdr; UINT error = CHANNEL_RC_OK; rdpdr = (rdpdrPlugin*) rdpdr_get_open_handle_data(openHandle); - if (!rdpdr) + if (!rdpdr || !pData) { WLog_ERR(TAG, "rdpdr_virtual_channel_open_event: error no match"); return; @@ -1275,17 +1257,17 @@ static VOID VCAPITYPE rdpdr_virtual_channel_open_event(DWORD openHandle, UINT ev switch (event) { - case CHANNEL_EVENT_DATA_RECEIVED: - if ((error = rdpdr_virtual_channel_event_data_received(rdpdr, pData, dataLength, totalLength, dataFlags))) - WLog_ERR(TAG, "rdpdr_virtual_channel_event_data_received failed with error %lu!", error ); - break; + case CHANNEL_EVENT_DATA_RECEIVED: + if ((error = rdpdr_virtual_channel_event_data_received(rdpdr, pData, dataLength, totalLength, dataFlags))) + WLog_ERR(TAG, "rdpdr_virtual_channel_event_data_received failed with error %lu!", error ); + break; - case CHANNEL_EVENT_WRITE_COMPLETE: - Stream_Free((wStream*) pData, TRUE); - break; + case CHANNEL_EVENT_WRITE_COMPLETE: + Stream_Free((wStream*) pData, TRUE); + break; - case CHANNEL_EVENT_USER: - break; + case CHANNEL_EVENT_USER: + break; } if (error && rdpdr->rdpcontext) setChannelError(rdpdr->rdpcontext, error, "rdpdr_virtual_channel_open_event reported an error"); @@ -1302,7 +1284,7 @@ static void* rdpdr_virtual_channel_client_thread(void* arg) if (!rdpdr) { - ExitThread((DWORD) ERROR_INVALID_PARAMETER); + ExitThread((DWORD) CHANNEL_RC_NULL_DATA); return NULL; } @@ -1354,20 +1336,17 @@ static UINT rdpdr_virtual_channel_event_connected(rdpdrPlugin* rdpdr, LPVOID pDa UINT32 status; UINT error; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - status = rdpdr->channelEntryPoints.pVirtualChannelOpen(rdpdr->InitHandle, - &rdpdr->OpenHandle, rdpdr->channelDef.name, rdpdr_virtual_channel_open_event); + &rdpdr->OpenHandle, rdpdr->channelDef.name, rdpdr_virtual_channel_open_event); - if (status != CHANNEL_RC_OK) - { - WLog_ERR(TAG, "pVirtualChannelOpen failed with %s [%08X]", - WTSErrorToString(status), status); - return status; - } + if (status != CHANNEL_RC_OK) + { + WLog_ERR(TAG, "pVirtualChannelOpen failed with %s [%08X]", + WTSErrorToString(status), status); + return status; + } - if ((error = rdpdr_add_open_handle_data(rdpdr->OpenHandle, rdpdr))) + if ((error = rdpdr_add_open_handle_data(rdpdr->OpenHandle, rdpdr))) { WLog_ERR(TAG, "rdpdr_add_open_handle_data failed with error %lu!", error); return error; @@ -1398,15 +1377,12 @@ static UINT rdpdr_virtual_channel_event_disconnected(rdpdrPlugin* rdpdr) { UINT error; - if (!rdpdr) - return ERROR_INVALID_PARAMETER; - if (MessageQueue_PostQuit(rdpdr->queue, 0) && (WaitForSingleObject(rdpdr->thread, INFINITE) == WAIT_FAILED)) - { - error = GetLastError(); - WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); - return error; - } + { + error = GetLastError(); + WLog_ERR(TAG, "WaitForSingleObject failed with error %lu!", error); + return error; + } MessageQueue_Free(rdpdr->queue); CloseHandle(rdpdr->thread); @@ -1415,10 +1391,10 @@ static UINT rdpdr_virtual_channel_event_disconnected(rdpdrPlugin* rdpdr) rdpdr->thread = NULL; if ((error = drive_hotplug_thread_terminate(rdpdr))) - { - WLog_ERR(TAG, "drive_hotplug_thread_terminate failed with error %lu!", error); - return error; - } + { + WLog_ERR(TAG, "drive_hotplug_thread_terminate failed with error %lu!", error); + return error; + } error = rdpdr->channelEntryPoints.pVirtualChannelClose(rdpdr->OpenHandle); if (CHANNEL_RC_OK != error) @@ -1465,25 +1441,25 @@ static VOID VCAPITYPE rdpdr_virtual_channel_init_event(LPVOID pInitHandle, UINT switch (event) { - case CHANNEL_EVENT_INITIALIZED: - break; - case CHANNEL_EVENT_CONNECTED: - if ((error = rdpdr_virtual_channel_event_connected(rdpdr, pData, dataLength))) - WLog_ERR(TAG, "rdpdr_virtual_channel_event_connected failed with error %lu!", error); - break; + case CHANNEL_EVENT_INITIALIZED: + break; + case CHANNEL_EVENT_CONNECTED: + if ((error = rdpdr_virtual_channel_event_connected(rdpdr, pData, dataLength))) + WLog_ERR(TAG, "rdpdr_virtual_channel_event_connected failed with error %lu!", error); + break; - case CHANNEL_EVENT_DISCONNECTED: - if ((error = rdpdr_virtual_channel_event_disconnected(rdpdr))) - WLog_ERR(TAG, "rdpdr_virtual_channel_event_disconnected failed with error %lu!", error); - break; + case CHANNEL_EVENT_DISCONNECTED: + if ((error = rdpdr_virtual_channel_event_disconnected(rdpdr))) + WLog_ERR(TAG, "rdpdr_virtual_channel_event_disconnected failed with error %lu!", error); + break; - case CHANNEL_EVENT_TERMINATED: - rdpdr_virtual_channel_event_terminated(rdpdr); - break; - default: - WLog_ERR(TAG, "unknown event %d!", event); - error = ERROR_INVALID_DATA; - break; + case CHANNEL_EVENT_TERMINATED: + rdpdr_virtual_channel_event_terminated(rdpdr); + break; + default: + WLog_ERR(TAG, "unknown event %d!", event); + error = ERROR_INVALID_DATA; + break; } if (error && rdpdr->rdpcontext) From 57f1e26f36af5c5bf02d21043617c9769dda0225 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 28 Jan 2016 11:12:20 +0100 Subject: [PATCH 3/7] Checking capability read return. Updated copyright headers. --- channels/rdpdr/client/rdpdr_capabilities.c | 11 ++++++----- channels/rdpdr/client/rdpdr_main.c | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index 4de411674..4a64f98b6 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -4,8 +4,9 @@ * * Copyright 2010-2011 Vic Lee * Copyright 2010-2012 Marc-Andre Moreau - * Copyright 2015 Thincast Technologies GmbH + * Copyright 2015-2016 Thincast Technologies GmbH * Copyright 2015 DI (FH) Martin Haimberger + * Copyright 2016 Armin Novak * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -203,19 +204,19 @@ UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) break; case CAP_PRINTER_TYPE: - rdpdr_process_printer_capset(rdpdr, s); + status = rdpdr_process_printer_capset(rdpdr, s); break; case CAP_PORT_TYPE: - rdpdr_process_port_capset(rdpdr, s); + status = rdpdr_process_port_capset(rdpdr, s); break; case CAP_DRIVE_TYPE: - rdpdr_process_drive_capset(rdpdr, s); + status = rdpdr_process_drive_capset(rdpdr, s); break; case CAP_SMARTCARD_TYPE: - rdpdr_process_smartcard_capset(rdpdr, s); + status = rdpdr_process_smartcard_capset(rdpdr, s); break; default: diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index ae097fa87..da26ca184 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -4,8 +4,9 @@ * * Copyright 2010-2011 Vic Lee * Copyright 2010-2012 Marc-Andre Moreau - * Copyright 2015 Thincast Technologies GmbH + * Copyright 2015-2016 Thincast Technologies GmbH * Copyright 2015 DI (FH) Martin Haimberger + * Copyright 2016 Armin Novak * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 6f50589c05cdda12c01eba3815a4f28c516eecf0 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 28 Jan 2016 12:05:14 +0100 Subject: [PATCH 4/7] Cleared up error code usage. --- channels/rdpdr/client/irp.c | 21 +++++++++++++++++++-- channels/rdpdr/client/irp.h | 2 +- channels/rdpdr/client/rdpdr_main.c | 11 +++++------ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/channels/rdpdr/client/irp.c b/channels/rdpdr/client/irp.c index 2d1f96506..899b89a9f 100644 --- a/channels/rdpdr/client/irp.c +++ b/channels/rdpdr/client/irp.c @@ -77,18 +77,28 @@ static UINT irp_complete(IRP* irp) return error; } -IRP* irp_new(DEVMAN* devman, wStream* s) +IRP* irp_new(DEVMAN* devman, wStream* s, UINT* error) { IRP* irp; DEVICE* device; UINT32 DeviceId; + if (!Stream_EnsureRemainingCapacity(s, 20)) + { + if (error) + *error = CHANNEL_RC_NO_BUFFER; + return NULL; + } + Stream_Read_UINT32(s, DeviceId); /* DeviceId (4 bytes) */ device = devman_get_device_by_id(devman, DeviceId); if (!device) { - WLog_ERR(TAG, "devman_get_device_by_id failed!"); + WLog_WARN(TAG, "devman_get_device_by_id failed!"); + if (error) + *error = CHANNEL_RC_OK; + return NULL; }; @@ -97,6 +107,8 @@ IRP* irp_new(DEVMAN* devman, wStream* s) if (!irp) { WLog_ERR(TAG, "_aligned_malloc failed!"); + if (error) + *error = CHANNEL_RC_NO_MEMORY; return NULL; } @@ -117,6 +129,8 @@ IRP* irp_new(DEVMAN* devman, wStream* s) { WLog_ERR(TAG, "Stream_New failed!"); _aligned_free(irp); + if (error) + *error = CHANNEL_RC_NO_MEMORY; return NULL; } Stream_Write_UINT16(irp->output, RDPDR_CTYP_CORE); /* Component (2 bytes) */ @@ -131,5 +145,8 @@ IRP* irp_new(DEVMAN* devman, wStream* s) irp->thread = NULL; irp->cancelled = FALSE; + if (error) + *error = CHANNEL_RC_OK; + return irp; } diff --git a/channels/rdpdr/client/irp.h b/channels/rdpdr/client/irp.h index d94366373..17d75acde 100644 --- a/channels/rdpdr/client/irp.h +++ b/channels/rdpdr/client/irp.h @@ -23,6 +23,6 @@ #include "rdpdr_main.h" -IRP* irp_new(DEVMAN* devman, wStream* s); +IRP* irp_new(DEVMAN* devman, wStream* s, UINT* error); #endif /* FREERDP_CHANNEL_RDPDR_CLIENT_IRP_H */ diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index da26ca184..3378f92bc 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -497,7 +497,6 @@ static UINT handle_hotplug(rdpdrPlugin* rdpdr) free(drive->Path); free(drive->Name); free(drive); - error = CHANNEL_RC_NO_MEMORY; goto cleanup; } } @@ -820,7 +819,7 @@ static UINT rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use if (!Stream_EnsureRemainingCapacity(s, 20 + data_len)) { WLog_ERR(TAG, "Stream_EnsureRemainingCapacity failed!"); - return CHANNEL_RC_NO_MEMORY; + return CHANNEL_RC_NO_BUFFER; } Stream_Write_UINT32(s, device->type); /* deviceType */ @@ -869,12 +868,12 @@ static UINT rdpdr_process_irp(rdpdrPlugin* rdpdr, wStream* s) IRP* irp; UINT error = CHANNEL_RC_OK; - irp = irp_new(rdpdr->devman, s); + irp = irp_new(rdpdr->devman, s, &error); if (!irp) { - WLog_ERR(TAG, "irp_new failed!"); - return CHANNEL_RC_NO_MEMORY; + WLog_ERR(TAG, "irp_new failed with %lu!", error); + return error; } IFCALLRET(irp->device->IRPRequest, error, irp->device, irp); @@ -1217,7 +1216,7 @@ static UINT rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, if (!Stream_EnsureRemainingCapacity(data_in, (int) dataLength)) { WLog_ERR(TAG, "Stream_EnsureRemainingCapacity failed!"); - return CHANNEL_RC_NO_MEMORY; + return CHANNEL_RC_NO_BUFFER; } Stream_Write(data_in, pData, dataLength); From d847993a0c3fb26147a3d652a97336a5656b5343 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 28 Jan 2016 12:25:44 +0100 Subject: [PATCH 5/7] Using Stream_ReminingLength for read checks. --- channels/rdpdr/client/irp.c | 2 +- channels/rdpdr/client/rdpdr_capabilities.c | 24 +++++++++++----------- channels/rdpdr/client/rdpdr_main.c | 10 ++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/channels/rdpdr/client/irp.c b/channels/rdpdr/client/irp.c index 899b89a9f..d98870d9d 100644 --- a/channels/rdpdr/client/irp.c +++ b/channels/rdpdr/client/irp.c @@ -83,7 +83,7 @@ IRP* irp_new(DEVMAN* devman, wStream* s, UINT* error) DEVICE* device; UINT32 DeviceId; - if (!Stream_EnsureRemainingCapacity(s, 20)) + if (Stream_GetRemainingLength(s) < 20) { if (error) *error = CHANNEL_RC_NO_BUFFER; diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index 4a64f98b6..4dba231e2 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -65,12 +65,12 @@ static UINT rdpdr_process_general_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; - if (!Stream_EnsureRemainingCapacity(s, 2)) + if (Stream_GetRemainingLength(s) < 2) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, capabilityLength); - if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + if (Stream_GetRemainingLength(s) < capabilityLength - 4) return CHANNEL_RC_NO_BUFFER; Stream_Seek(s, capabilityLength - 4); @@ -89,12 +89,12 @@ static UINT rdpdr_process_printer_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; - if (!Stream_EnsureRemainingCapacity(s, 2)) + if (Stream_GetRemainingLength(s) < 2) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, capabilityLength); - if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + if (Stream_GetRemainingLength(s) < capabilityLength - 4) return CHANNEL_RC_NO_BUFFER; Stream_Seek(s, capabilityLength - 4); @@ -113,12 +113,12 @@ static UINT rdpdr_process_port_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; - if (!Stream_EnsureRemainingCapacity(s, 2)) + if (Stream_GetRemainingLength(s) < 2) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, capabilityLength); - if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + if (Stream_GetRemainingLength(s) < capabilityLength - 4) return CHANNEL_RC_NO_BUFFER; Stream_Seek(s, capabilityLength - 4); @@ -137,12 +137,12 @@ static UINT rdpdr_process_drive_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; - if (!Stream_EnsureRemainingCapacity(s, 2)) + if (Stream_GetRemainingLength(s) < 2) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, capabilityLength); - if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + if (Stream_GetRemainingLength(s) < capabilityLength - 4) return CHANNEL_RC_NO_BUFFER; Stream_Seek(s, capabilityLength - 4); @@ -161,12 +161,12 @@ static UINT rdpdr_process_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) { UINT16 capabilityLength; - if (!Stream_EnsureRemainingCapacity(s, 2)) + if (Stream_GetRemainingLength(s) < 2) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, capabilityLength); - if (!Stream_EnsureRemainingCapacity(s, capabilityLength - 4)) + if (Stream_GetRemainingLength(s) < capabilityLength - 4) return CHANNEL_RC_NO_BUFFER; Stream_Seek(s, capabilityLength - 4); @@ -184,13 +184,13 @@ UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) if (!rdpdr || !s) return CHANNEL_RC_NULL_DATA; - if (!Stream_EnsureRemainingCapacity(s, 4)) + if (Stream_GetRemainingLength(s) < 4) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, numCapabilities); Stream_Seek(s, 2); /* pad (2 bytes) */ - if (!Stream_EnsureRemainingCapacity(s, sizeof(UINT16) * numCapabilities)) + if (Stream_GetRemainingLength(s) < sizeof(UINT16) * numCapabilities) return CHANNEL_RC_NO_BUFFER; for (i = 0; i < numCapabilities; i++) diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 3378f92bc..62da00b51 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -664,7 +664,7 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) static UINT rdpdr_process_server_announce_request(rdpdrPlugin* rdpdr, wStream* s) { - if (!Stream_EnsureRemainingCapacity(s, 8)) + if (Stream_GetRemainingLength(s) < 8) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, rdpdr->versionMajor); @@ -745,7 +745,7 @@ static UINT rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s UINT16 versionMinor; UINT32 clientID; - if (!Stream_EnsureRemainingCapacity(s, 8)) + if (Stream_GetRemainingLength(s) < 8) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, versionMajor); @@ -933,7 +933,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) if (!rdpdr || !s) return CHANNEL_RC_NULL_DATA; - if (!Stream_EnsureRemainingCapacity(s, 4)) + if (Stream_GetRemainingLength(s) < 4) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT16(s, component); /* Component (2 bytes) */ @@ -994,7 +994,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) case PAKID_CORE_DEVICE_REPLY: /* connect to a specific resource */ - if (Stream_EnsureRemainingCapacity(s, 8)) + if (Stream_GetRemainingLength(s) < 8) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT32(s, deviceId); @@ -1024,7 +1024,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) case PAKID_PRN_CACHE_DATA: { UINT32 eventID; - if (Stream_EnsureRemainingCapacity(s, 4)) + if (Stream_GetRemainingLength(s) < 4) return CHANNEL_RC_NO_BUFFER; Stream_Read_UINT32(s, eventID); From 96aecca394fc7ac72e6796b31c8950f0223c9f77 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 2 Feb 2016 17:59:56 +0100 Subject: [PATCH 6/7] Fixed length check issue. --- channels/rdpdr/client/rdpdr_capabilities.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index 4dba231e2..ad216e6f2 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -190,11 +190,11 @@ UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) Stream_Read_UINT16(s, numCapabilities); Stream_Seek(s, 2); /* pad (2 bytes) */ - if (Stream_GetRemainingLength(s) < sizeof(UINT16) * numCapabilities) - return CHANNEL_RC_NO_BUFFER; - for (i = 0; i < numCapabilities; i++) { + if (Stream_GetRemainingLength(s) < sizeof(UINT16)) + return CHANNEL_RC_NO_BUFFER; + Stream_Read_UINT16(s, capabilityType); switch (capabilityType) From 511f9e810a1aeff438ccd79fc4a4ccfa4d1e018a Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 3 Feb 2016 11:04:35 +0100 Subject: [PATCH 7/7] Use ERROR_INVALID_DATA for short buffers. --- channels/rdpdr/client/irp.c | 2 +- channels/rdpdr/client/rdpdr_capabilities.c | 24 +++++++++++----------- channels/rdpdr/client/rdpdr_main.c | 14 ++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/channels/rdpdr/client/irp.c b/channels/rdpdr/client/irp.c index d98870d9d..d8eaece36 100644 --- a/channels/rdpdr/client/irp.c +++ b/channels/rdpdr/client/irp.c @@ -86,7 +86,7 @@ IRP* irp_new(DEVMAN* devman, wStream* s, UINT* error) if (Stream_GetRemainingLength(s) < 20) { if (error) - *error = CHANNEL_RC_NO_BUFFER; + *error = ERROR_INVALID_DATA; return NULL; } diff --git a/channels/rdpdr/client/rdpdr_capabilities.c b/channels/rdpdr/client/rdpdr_capabilities.c index ad216e6f2..4ff3ef594 100644 --- a/channels/rdpdr/client/rdpdr_capabilities.c +++ b/channels/rdpdr/client/rdpdr_capabilities.c @@ -66,12 +66,12 @@ static UINT rdpdr_process_general_capset(rdpdrPlugin* rdpdr, wStream* s) UINT16 capabilityLength; if (Stream_GetRemainingLength(s) < 2) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityLength); if (Stream_GetRemainingLength(s) < capabilityLength - 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Seek(s, capabilityLength - 4); @@ -90,12 +90,12 @@ static UINT rdpdr_process_printer_capset(rdpdrPlugin* rdpdr, wStream* s) UINT16 capabilityLength; if (Stream_GetRemainingLength(s) < 2) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityLength); if (Stream_GetRemainingLength(s) < capabilityLength - 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Seek(s, capabilityLength - 4); @@ -114,12 +114,12 @@ static UINT rdpdr_process_port_capset(rdpdrPlugin* rdpdr, wStream* s) UINT16 capabilityLength; if (Stream_GetRemainingLength(s) < 2) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityLength); if (Stream_GetRemainingLength(s) < capabilityLength - 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Seek(s, capabilityLength - 4); @@ -138,12 +138,12 @@ static UINT rdpdr_process_drive_capset(rdpdrPlugin* rdpdr, wStream* s) UINT16 capabilityLength; if (Stream_GetRemainingLength(s) < 2) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityLength); if (Stream_GetRemainingLength(s) < capabilityLength - 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Seek(s, capabilityLength - 4); @@ -162,12 +162,12 @@ static UINT rdpdr_process_smartcard_capset(rdpdrPlugin* rdpdr, wStream* s) UINT16 capabilityLength; if (Stream_GetRemainingLength(s) < 2) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityLength); if (Stream_GetRemainingLength(s) < capabilityLength - 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Seek(s, capabilityLength - 4); @@ -185,7 +185,7 @@ UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) return CHANNEL_RC_NULL_DATA; if (Stream_GetRemainingLength(s) < 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, numCapabilities); Stream_Seek(s, 2); /* pad (2 bytes) */ @@ -193,7 +193,7 @@ UINT rdpdr_process_capability_request(rdpdrPlugin* rdpdr, wStream* s) for (i = 0; i < numCapabilities; i++) { if (Stream_GetRemainingLength(s) < sizeof(UINT16)) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, capabilityType); diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 62da00b51..87ee6fbdb 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -665,7 +665,7 @@ static UINT rdpdr_process_connect(rdpdrPlugin* rdpdr) static UINT rdpdr_process_server_announce_request(rdpdrPlugin* rdpdr, wStream* s) { if (Stream_GetRemainingLength(s) < 8) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, rdpdr->versionMajor); Stream_Read_UINT16(s, rdpdr->versionMinor); @@ -746,7 +746,7 @@ static UINT rdpdr_process_server_clientid_confirm(rdpdrPlugin* rdpdr, wStream* s UINT32 clientID; if (Stream_GetRemainingLength(s) < 8) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, versionMajor); Stream_Read_UINT16(s, versionMinor); @@ -819,7 +819,7 @@ static UINT rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use if (!Stream_EnsureRemainingCapacity(s, 20 + data_len)) { WLog_ERR(TAG, "Stream_EnsureRemainingCapacity failed!"); - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; } Stream_Write_UINT32(s, device->type); /* deviceType */ @@ -934,7 +934,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) return CHANNEL_RC_NULL_DATA; if (Stream_GetRemainingLength(s) < 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT16(s, component); /* Component (2 bytes) */ Stream_Read_UINT16(s, packetId); /* PacketId (2 bytes) */ @@ -995,7 +995,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) case PAKID_CORE_DEVICE_REPLY: /* connect to a specific resource */ if (Stream_GetRemainingLength(s) < 8) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT32(s, deviceId); Stream_Read_UINT32(s, status); @@ -1025,7 +1025,7 @@ static UINT rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) { UINT32 eventID; if (Stream_GetRemainingLength(s) < 4) - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; Stream_Read_UINT32(s, eventID); WLog_ERR(TAG, "Ignoring unhandled message PAKID_PRN_CACHE_DATA (EventID: 0x%04X)", eventID); @@ -1216,7 +1216,7 @@ static UINT rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, if (!Stream_EnsureRemainingCapacity(data_in, (int) dataLength)) { WLog_ERR(TAG, "Stream_EnsureRemainingCapacity failed!"); - return CHANNEL_RC_NO_BUFFER; + return ERROR_INVALID_DATA; } Stream_Write(data_in, pData, dataLength);