From 870b7025b77c59addde837a6946cf33947ee21d2 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 16 Nov 2018 15:41:19 +0100 Subject: [PATCH] Improved error mapping and tightened checks in rdg_process_packet --- libfreerdp/core/gateway/rdg.c | 129 ++++++++++++++++++++++++--- libfreerdp/core/gateway/rpc_client.c | 2 +- libfreerdp/core/gateway/rpc_fault.c | 29 +++--- libfreerdp/core/gateway/rpc_fault.h | 5 +- 4 files changed, 137 insertions(+), 28 deletions(-) diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c index 8dd765a66..a3ed42080 100644 --- a/libfreerdp/core/gateway/rdg.c +++ b/libfreerdp/core/gateway/rdg.c @@ -37,6 +37,7 @@ #include "../proxy.h" #include "../rdp.h" #include "../../crypto/opensslcompat.h" +#include "rpc_fault.h" #define TAG FREERDP_TAG("core.gateway.rdg") @@ -147,6 +148,70 @@ typedef struct rdg_packet_header #pragma pack(pop) +typedef struct +{ + UINT32 code; + const char* name; +} t_err_mapping; + +static const t_err_mapping fields_present[] = +{ + {HTTP_CHANNEL_RESPONSE_FIELD_CHANNELID, "HTTP_CHANNEL_RESPONSE_FIELD_CHANNELID"}, + {HTTP_CHANNEL_RESPONSE_OPTIONAL, "HTTP_CHANNEL_RESPONSE_OPTIONAL"}, + {HTTP_CHANNEL_RESPONSE_FIELD_UDPPORT, "HTTP_CHANNEL_RESPONSE_FIELD_UDPPORT"} +}; + +static const t_err_mapping extended_auth[] = +{ + {HTTP_EXTENDED_AUTH_NONE, "HTTP_EXTENDED_AUTH_NONE"}, + {HTTP_EXTENDED_AUTH_SC, "HTTP_EXTENDED_AUTH_SC"}, + {HTTP_EXTENDED_AUTH_PAA, "HTTP_EXTENDED_AUTH_PAA"}, + {HTTP_EXTENDED_AUTH_SSPI_NTLM, "HTTP_EXTENDED_AUTH_SSPI_NTLM"} +}; + +static const char* fields_present_to_string(UINT16 fieldsPresent) +{ + size_t x = 0; + static char buffer[1024] = { 0 }; + char fields[12]; + + for (x = 0; x < ARRAYSIZE(fields_present); x++) + { + if (strnlen(buffer, ARRAYSIZE(buffer)) > 0) + strcat(buffer, "|"); + + if ((fields_present[x].code & fieldsPresent) != 0) + strcat(buffer, fields_present[x].name); + } + + sprintf_s(fields, ARRAYSIZE(fields), " [%04"PRIx16"]", fieldsPresent); + strcat(buffer, fields); + return buffer; +} + +static const char* extended_auth_to_string(UINT16 auth) +{ + size_t x = 0; + static char buffer[1024] = { 0 }; + char fields[12]; + + if (auth == HTTP_EXTENDED_AUTH_NONE) + return "HTTP_EXTENDED_AUTH_NONE [0x0000]"; + + for (x = 0; x < ARRAYSIZE(extended_auth); x++) + { + if (strnlen(buffer, ARRAYSIZE(buffer)) > 0) + strcat(buffer, "|"); + + if ((extended_auth[x].code & auth) != 0) + strcat(buffer, extended_auth[x].name); + } + + sprintf_s(fields, ARRAYSIZE(fields), " [%04"PRIx16"]", auth); + strcat(buffer, fields); + return buffer; +} + static BOOL rdg_write_packet(rdpRdg* rdg, wStream* sPacket) { size_t s; @@ -546,6 +611,9 @@ static BOOL rdg_skip_seed_payload(rdpTls* tls, SSIZE_T lastResponseLength) static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s) { UINT32 errorCode; + UINT16 serverVersion, extendedAuth; + BYTE verMajor, verMinor; + const char* error; WLog_DBG(TAG, "Handshake response received"); if (rdg->state != RDG_CLIENT_STATE_HANDSHAKE) @@ -553,15 +621,22 @@ static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s) return FALSE; } - if (Stream_GetRemainingLength(s) < 12) + if (Stream_GetRemainingLength(s) < 10) return FALSE; - Stream_Seek(s, 8); Stream_Read_UINT32(s, errorCode); + Stream_Read_UINT8(s, verMajor); + Stream_Read_UINT8(s, verMinor); + Stream_Read_UINT16(s, serverVersion); + Stream_Read_UINT16(s, extendedAuth); + error = rpc_error_to_string(errorCode); + WLog_DBG(TAG, + "errorCode=%s, verMajor=%"PRId8", verMinor=%"PRId8", serverVersion=%"PRId16", extendedAuth=%s", + error, verMajor, verMinor, serverVersion, extended_auth_to_string(extendedAuth)); if (FAILED(errorCode)) { - WLog_DBG(TAG, "Handshake error %"PRIx32, (HRESULT)errorCode); + WLog_DBG(TAG, "Handshake error %s", error); return FALSE; } @@ -570,7 +645,9 @@ static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s) static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s) { + UINT16 serverVersion, fieldsPresent; UINT32 errorCode; + const char* error; WLog_DBG(TAG, "Tunnel response received"); if (rdg->state != RDG_CLIENT_STATE_TUNNEL_CREATE) @@ -578,15 +655,20 @@ static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s) return FALSE; } - if (Stream_GetRemainingLength(s) < 14) + if (Stream_GetRemainingLength(s) < 10) return FALSE; - Stream_Seek(s, 10); + Stream_Read_UINT16(s, serverVersion); Stream_Read_UINT32(s, errorCode); + Stream_Read_UINT16(s, fieldsPresent); + Stream_Seek_UINT16(s); /* reserved */ + error = rpc_error_to_string(errorCode); + WLog_DBG(TAG, "serverVersion=%"PRId16", errorCode=%s, fieldsPresent=%s", + serverVersion, error, fields_present_to_string(fieldsPresent)); if (FAILED(errorCode)) { - WLog_DBG(TAG, "Tunnel creation error %"PRIx32, (HRESULT)errorCode); + WLog_DBG(TAG, "Tunnel creation error %s", error); return FALSE; } @@ -596,6 +678,8 @@ static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s) static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s) { UINT32 errorCode; + UINT16 fieldsPresent; + const char* error; WLog_DBG(TAG, "Tunnel authorization received"); if (rdg->state != RDG_CLIENT_STATE_TUNNEL_AUTHORIZE) @@ -603,15 +687,19 @@ static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s) return FALSE; } - if (Stream_GetRemainingLength(s) < 12) + if (Stream_GetRemainingLength(s) < 8) return FALSE; - Stream_Seek(s, 8); Stream_Read_UINT32(s, errorCode); + Stream_Read_UINT16(s, fieldsPresent); + Stream_Seek_UINT16(s); /* reserved */ + error = rpc_error_to_string(errorCode); + WLog_DBG(TAG, "errorCode=%s, fieldsPresent=%s", + error, fields_present_to_string(fieldsPresent)); if (FAILED(errorCode)) { - WLog_DBG(TAG, "Tunnel authorization error %"PRIx32, (HRESULT)errorCode); + WLog_DBG(TAG, "Tunnel authorization error %s", error); return FALSE; } @@ -620,7 +708,9 @@ static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s) static BOOL rdg_process_channel_response(rdpRdg* rdg, wStream* s) { + UINT16 fieldsPresent; UINT32 errorCode; + const char* error; WLog_DBG(TAG, "Channel response received"); if (rdg->state != RDG_CLIENT_STATE_CHANNEL_CREATE) @@ -628,15 +718,20 @@ static BOOL rdg_process_channel_response(rdpRdg* rdg, wStream* s) return FALSE; } - if (Stream_GetRemainingLength(s) < 12) + if (Stream_GetRemainingLength(s) < 8) return FALSE; - Stream_Seek(s, 8); Stream_Read_UINT32(s, errorCode); + Stream_Read_UINT16(s, fieldsPresent); + Stream_Seek_UINT16(s); /* reserved */ + error = rpc_error_to_string(errorCode); + WLog_DBG(TAG, "channel response errorCode=%s, fieldsPresent=%s", + error, fields_present_to_string(fieldsPresent)); if (FAILED(errorCode)) { - WLog_DBG(TAG, "Channel error %"PRIx32, (HRESULT)errorCode); + WLog_ERR(TAG, "channel response errorCode=%s, fieldsPresent=%s", + error, fields_present_to_string(fieldsPresent)); return FALSE; } @@ -648,12 +743,18 @@ static BOOL rdg_process_packet(rdpRdg* rdg, wStream* s) { BOOL status = TRUE; UINT16 type; + UINT32 packetLength; Stream_SetPosition(s, 0); - if (Stream_GetRemainingLength(s) < 2) + if (Stream_GetRemainingLength(s) < 8) return FALSE; - Stream_Peek_UINT16(s, type); + Stream_Read_UINT16(s, type); + Stream_Seek_UINT16(s); /* reserved */ + Stream_Read_UINT32(s, packetLength); + + if (Stream_GetRemainingLength(s) < packetLength) + return FALSE; switch (type) { diff --git a/libfreerdp/core/gateway/rpc_client.c b/libfreerdp/core/gateway/rpc_client.c index cd0739a48..382296b49 100644 --- a/libfreerdp/core/gateway/rpc_client.c +++ b/libfreerdp/core/gateway/rpc_client.c @@ -446,7 +446,7 @@ static int rpc_client_recv_fragment(rdpRpc* rpc, wStream* fragment) } else if (header->common.ptype == PTYPE_FAULT) { - rpc_recv_fault_pdu(header); + rpc_recv_fault_pdu(header->fault.status); return -1; } else diff --git a/libfreerdp/core/gateway/rpc_fault.c b/libfreerdp/core/gateway/rpc_fault.c index 1e55e2b6e..274f26821 100644 --- a/libfreerdp/core/gateway/rpc_fault.c +++ b/libfreerdp/core/gateway/rpc_fault.c @@ -25,6 +25,8 @@ #include "rpc_fault.h" +#include "rpc.h" + #define TAG FREERDP_TAG("core.gateway.rpc") static const RPC_FAULT_CODE RPC_FAULT_CODES[] = @@ -359,19 +361,17 @@ static UINT32 rpc_map_status_code_to_win32_error_code(UINT32 code) return code; } -int rpc_recv_fault_pdu(rpcconn_hdr_t* header) +const char* rpc_error_to_string(UINT32 code) { - int index; - UINT32 code; - WLog_ERR(TAG, "RPC Fault PDU:"); - code = rpc_map_status_code_to_win32_error_code(header->fault.status); + size_t index; + static char buffer[1024]; for (index = 0; RPC_FAULT_CODES[index].name != NULL; index++) { if (RPC_FAULT_CODES[index].code == code) { - WLog_ERR(TAG, "status: %s (0x%08"PRIX32")", RPC_FAULT_CODES[index].name, code); - return 0; + sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", RPC_FAULT_CODES[index].name, code); + goto out; } } @@ -379,11 +379,20 @@ int rpc_recv_fault_pdu(rpcconn_hdr_t* header) { if (RPC_TSG_FAULT_CODES[index].code == code) { - WLog_ERR(TAG, "status: %s (0x%08"PRIX32")", RPC_TSG_FAULT_CODES[index].name, code); - return 0; + sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", RPC_TSG_FAULT_CODES[index].name, code); + goto out; } } - WLog_ERR(TAG, "status: %s (0x%08"PRIX32")", "UNKNOWN", code); + sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", "UNKNOWN", code); +out: + return buffer; +} + + +int rpc_recv_fault_pdu(UINT32 status) +{ + UINT32 code = rpc_map_status_code_to_win32_error_code(status); + WLog_ERR(TAG, "RPC Fault PDU: status=%s", rpc_error_to_string(code)); return 0; } diff --git a/libfreerdp/core/gateway/rpc_fault.h b/libfreerdp/core/gateway/rpc_fault.h index a93359e4c..bfdc0889c 100644 --- a/libfreerdp/core/gateway/rpc_fault.h +++ b/libfreerdp/core/gateway/rpc_fault.h @@ -20,11 +20,10 @@ #ifndef FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H #define FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H -#include "rpc.h" - #include #include -FREERDP_LOCAL int rpc_recv_fault_pdu(rpcconn_hdr_t* header); +FREERDP_LOCAL int rpc_recv_fault_pdu(UINT32 status); +FREERDP_LOCAL const char* rpc_error_to_string(UINT32 error); #endif /* FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H */