From 7e397d0f1ca693f4c5b8f7de4b0dd978ca86033d Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 18 Oct 2018 16:58:53 +0200 Subject: [PATCH 1/4] Fixed http gateway body length read. --- libfreerdp/core/gateway/http.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index 6920d98ff..565c878a2 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -842,15 +842,15 @@ HttpResponse* http_response_recv(rdpTls* tls) } if (response->BodyLength > 0) - { response->BodyContent = &(Stream_Buffer(response->data))[payloadOffset]; - response->BodyLength = bodyLength; - } if (bodyLength != response->BodyLength) { WLog_WARN(TAG, "http_response_recv: %s unexpected body length: actual: %d, expected: %d", - response->ContentType, bodyLength, response->BodyLength); + response->ContentType, response->BodyLength, bodyLength); + + if (bodyLength > 0) + response->BodyLength = MIN(bodyLength, response->BodyLength); } } From d05684a50a4396b72024ce4d31379d1d34592079 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 19 Oct 2018 11:39:22 +0200 Subject: [PATCH 2/4] Properly parse ContentType to find length. --- libfreerdp/core/gateway/http.c | 58 ++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index 565c878a2..85248ede4 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -691,6 +691,41 @@ BOOL http_response_print(HttpResponse* response) return TRUE; } +static BOOL http_use_content_length(const char* cur) +{ + size_t pos = 0; + + if (!cur) + return FALSE; + + if (_strnicmp(cur, "application/rpc", 15) == 0) + pos = 15; + else if (_strnicmp(cur, "text/plain", 10) == 0) + pos = 10; + else if (_strnicmp(cur, "text/html", 9) == 0) + pos = 9; + + if (pos > 0) + { + char end = cur[pos]; + + switch (end) + { + case ' ': + case ';': + case '\0': + case '\r': + case '\n': + return TRUE; + + default: + return FALSE; + } + } + + return FALSE; +} + HttpResponse* http_response_recv(rdpTls* tls) { size_t size; @@ -793,19 +828,22 @@ HttpResponse* http_response_recv(rdpTls* tls) response->BodyLength = Stream_GetPosition(response->data) - payloadOffset; bodyLength = 0; /* expected body length */ - - if (response->ContentType) { - if (_stricmp(response->ContentType, "application/rpc") != 0) - bodyLength = response->ContentLength; - else if (_stricmp(response->ContentType, "text/plain") == 0) - bodyLength = response->ContentLength; - else if (_stricmp(response->ContentType, "text/html") == 0) - bodyLength = response->ContentLength; - } - else + const char* cur = response->ContentType; bodyLength = response->BodyLength; + while (cur != NULL) + { + if (http_use_content_length(cur)) + { + bodyLength = response->ContentLength; + break; + } + + cur = strchr(cur, ';'); + } + } + if (bodyLength > RESPONSE_SIZE_LIMIT) { WLog_ERR(TAG, "Expected request body too large! (%"PRIdz" bytes) Aborting!", bodyLength); From 5a747b118d68e4a81795506a6173b8e2a846b832 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 19 Oct 2018 11:59:28 +0200 Subject: [PATCH 3/4] Read http request in 4byte chunks until '\r\n\r\n' is found Avoid reading too much data in a single call to BIO_read as some implementations may return a lot more data than is part of the response. --- libfreerdp/core/gateway/http.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index 85248ede4..e862d35d1 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -742,10 +742,12 @@ HttpResponse* http_response_recv(rdpTls* tls) response->ContentLength = 0; - while (!payloadOffset) + while (payloadOffset == 0) { - int status = BIO_read(tls->bio, Stream_Pointer(response->data), - Stream_Capacity(response->data) - Stream_GetPosition(response->data)); + size_t s; + char* end; + /* Read until we encounter \r\n\r\n */ + int status = BIO_read(tls->bio, Stream_Pointer(response->data), 4); if (status <= 0) { @@ -761,28 +763,26 @@ HttpResponse* http_response_recv(rdpTls* tls) #endif Stream_Seek(response->data, (size_t)status); - if (Stream_GetRemainingLength(response->data) < 1024) - { - if (!Stream_EnsureRemainingCapacity(response->data, 1024)) - goto out_error; - } + if (!Stream_EnsureRemainingCapacity(response->data, 1024)) + goto out_error; position = Stream_GetPosition(response->data); - if (position > RESPONSE_SIZE_LIMIT) + if (position < 4) + continue; + else if (position > RESPONSE_SIZE_LIMIT) { WLog_ERR(TAG, "Request header too large! (%"PRIdz" bytes) Aborting!", bodyLength); goto out_error; } - if (position >= 4) - { - char* buffer = (char*)Stream_Buffer(response->data); - const char* line = string_strnstr(buffer, "\r\n\r\n", position); + /* Always check at most the lase 8 bytes for occurance of the desired + * sequence of \r\n\r\n */ + s = (position > 8) ? 8 : position; + end = (char*)Stream_Pointer(response->data) - s; - if (line) - payloadOffset = (line - buffer) + 4UL; - } + if (string_strnstr(end, "\r\n\r\n", s) != NULL) + payloadOffset = Stream_GetPosition(response->data); } if (payloadOffset) From b9933e7af48e083b8e2c05a59852d7586ac034d1 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Fri, 19 Oct 2018 12:52:14 +0200 Subject: [PATCH 4/4] Read byte by byte, the alignment may otherwise be broken. --- libfreerdp/core/gateway/http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index e862d35d1..ee17858ac 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -747,7 +747,7 @@ HttpResponse* http_response_recv(rdpTls* tls) size_t s; char* end; /* Read until we encounter \r\n\r\n */ - int status = BIO_read(tls->bio, Stream_Pointer(response->data), 4); + int status = BIO_read(tls->bio, Stream_Pointer(response->data), 1); if (status <= 0) {