File xrdp-cve-2020-4044-fix-0.patch of Package xrdp.27289
From e593f58a82bf79b556601ae08e9e25e366a662fb Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Fri, 12 Jun 2020 09:56:47 +0100
Subject: [PATCH] Fix for CVE-2020-4044
Reported by: Ashley Newson
---
sesman/libscp/libscp_types.h | 4 +
sesman/libscp/libscp_v0.c | 335 ++++++++++++++++++++------------
sesman/libscp/libscp_v1s.c | 336 +++++++++++++++++++++++----------
sesman/libscp/libscp_v1s_mng.c | 164 +++++++++++++---
sesman/scp.c | 12 +-
5 files changed, 592 insertions(+), 259 deletions(-)
Index: xrdp-0.9.6/sesman/libscp/libscp_types.h
===================================================================
--- xrdp-0.9.6.orig/sesman/libscp/libscp_types.h
+++ xrdp-0.9.6/sesman/libscp/libscp_types.h
@@ -59,6 +59,10 @@
#include "libscp_types_mng.h"
+/* Max server incoming and outgoing message size, used to stop memory
+ exhaustion attempts (CVE-2020-4044) */
+#define SCP_MAX_MESSAGE_SIZE 8192
+
struct SCP_CONNECTION
{
int in_sck;
Index: xrdp-0.9.6/sesman/libscp/libscp_v0.c
===================================================================
--- xrdp-0.9.6.orig/sesman/libscp/libscp_v0.c
+++ xrdp-0.9.6/sesman/libscp/libscp_v0.c
@@ -34,6 +34,65 @@
extern struct log_config *s_log;
+/** Maximum length of a string (two bytes + len), excluding the terminator
+ *
+ * Practially this is limited by [MS-RDPBCGR] TS_INFO_PACKET
+ * */
+#define STRING16_MAX_LEN 512
+
+/**
+ * Reads a big-endian uint16 followed by a string into a buffer
+ *
+ * Buffer is null-terminated on success
+ *
+ * @param s Input stream
+ * @param [out] Output buffer (must be >= (STRING16_MAX_LEN+1) chars)
+ * @param param Parameter we're reading
+ * @param line Line number reference
+ * @return != 0 if string read OK
+ */
+static
+int in_string16(struct stream *s, char str[], const char *param, int line)
+{
+ int result;
+
+ if (!s_check_rem(s, 2))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: %s len missing",
+ line, param);
+ result = 0;
+ }
+ else
+ {
+ unsigned int sz;
+
+ in_uint16_be(s, sz);
+ if (sz > STRING16_MAX_LEN)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: %s too long (%u chars)",
+ line, param, sz);
+ result = 0;
+ }
+ else
+ {
+ result = s_check_rem(s, sz);
+ if (!result)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: %s data missing",
+ line, param);
+ }
+ else
+ {
+ in_uint8a(s, str, sz);
+ str[sz] = '\0';
+ }
+ }
+ }
+ return result;
+}
/* client API */
/******************************************************************************/
enum SCP_CLIENT_STATES_E
@@ -71,10 +130,24 @@ scp_v0c_connect(struct SCP_CONNECTION *c
}
sz = g_strlen(s->username);
+ if (sz > STRING16_MAX_LEN)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: username too long",
+ __LINE__);
+ return SCP_CLIENT_STATE_SIZE_ERR;
+ }
out_uint16_be(c->out_s, sz);
out_uint8a(c->out_s, s->username, sz);
sz = g_strlen(s->password);
+ if (sz > STRING16_MAX_LEN)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: password too long",
+ __LINE__);
+ return SCP_CLIENT_STATE_SIZE_ERR;
+ }
out_uint16_be(c->out_s, sz);
out_uint8a(c->out_s, s->password, sz);
out_uint16_be(c->out_s, s->width);
@@ -111,14 +184,16 @@ scp_v0c_connect(struct SCP_CONNECTION *c
in_uint32_be(c->in_s, size);
- if (size < 14)
+ if (size < (8 + 2 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
{
- log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: packet size error", __LINE__);
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: msg size = %u",
+ __LINE__, (unsigned int)size);
return SCP_CLIENT_STATE_SIZE_ERR;
}
/* getting payload */
- init_stream(c->in_s, c->in_s->size);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, size - 8))
{
@@ -126,6 +201,8 @@ scp_v0c_connect(struct SCP_CONNECTION *c
return SCP_CLIENT_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
/* check code */
in_uint16_be(c->in_s, sz);
@@ -151,43 +228,38 @@ scp_v0c_connect(struct SCP_CONNECTION *c
return SCP_CLIENT_STATE_END;
}
-/* server API */
-/******************************************************************************/
-enum SCP_SERVER_STATES_E
-scp_v0s_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s, int skipVchk)
+/**
+ * Initialises a V0 session object
+ *
+ * At the time of the call, the version has been read from the connection
+ *
+ * @param c Connection
+ * @param [out] session pre-allocated session object
+ * @return SCP_SERVER_STATE_OK for success
+ */
+static enum SCP_SERVER_STATES_E
+scp_v0s_init_session(struct SCP_CONNECTION *c, struct SCP_SESSION *session)
{
- tui32 version = 0;
tui32 size;
- struct SCP_SESSION *session = 0;
- tui16 sz;
+ tui16 height;
+ tui16 width;
+ tui16 bpp;
tui32 code = 0;
- char *buf = 0;
+ char buf[STRING16_MAX_LEN + 1];
- if (!skipVchk)
- {
- LOG_DBG("[v0:%d] starting connection", __LINE__);
-
- if (0 == scp_tcp_force_recv(c->in_sck, c->in_s->data, 8))
- {
- c->in_s->end = c->in_s->data + 8;
- in_uint32_be(c->in_s, version);
-
- if (version != 0)
- {
- log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: version error", __LINE__);
- return SCP_SERVER_STATE_VERSION_ERR;
- }
- }
- else
- {
- log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__);
- return SCP_SERVER_STATE_NETWORK_ERR;
- }
- }
+ scp_session_set_version(session, 0);
+ /* Check for a header and a code value in the length */
in_uint32_be(c->in_s, size);
+ if (size < (8 + 2) || size > SCP_MAX_MESSAGE_SIZE)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: msg size = %u",
+ __LINE__, (unsigned int)size);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
- init_stream(c->in_s, 8196);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, size - 8))
{
@@ -201,16 +273,6 @@ scp_v0s_accept(struct SCP_CONNECTION *c,
if (code == 0 || code == 10 || code == 20)
{
- session = scp_session_create();
-
- if (0 == session)
- {
- log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__);
- return SCP_SERVER_STATE_INTERNAL_ERR;
- }
-
- scp_session_set_version(session, version);
-
if (code == 0)
{
scp_session_set_type(session, SCP_SESSION_TYPE_XVNC);
@@ -225,154 +287,130 @@ scp_v0s_accept(struct SCP_CONNECTION *c,
}
/* reading username */
- in_uint16_be(c->in_s, sz);
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
+ if (!in_string16(c->in_s, buf, "username", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_username(session, buf))
{
- scp_session_destroy(session);
log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: error setting username", __LINE__);
- g_free(buf);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- g_free(buf);
/* reading password */
- in_uint16_be(c->in_s, sz);
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
+ if (!in_string16(c->in_s, buf, "passwd", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_password(session, buf))
{
- scp_session_destroy(session);
log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: error setting password", __LINE__);
- g_free(buf);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- g_free(buf);
- /* width */
- in_uint16_be(c->in_s, sz);
- scp_session_set_width(session, sz);
- /* height */
- in_uint16_be(c->in_s, sz);
- scp_session_set_height(session, sz);
- /* bpp */
- in_uint16_be(c->in_s, sz);
- if (0 != scp_session_set_bpp(session, (tui8)sz))
+ /* width + height + bpp */
+ if (!s_check_rem(c->in_s, 2 + 2 + 2))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: width+height+bpp missing",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+ in_uint16_be(c->in_s, width);
+ scp_session_set_width(session, width);
+ in_uint16_be(c->in_s, height);
+ scp_session_set_height(session, height);
+ in_uint16_be(c->in_s, bpp);
+ if (0 != scp_session_set_bpp(session, (tui8)bpp))
{
- scp_session_destroy(session);
log_message(LOG_LEVEL_WARNING,
"[v0:%d] connection aborted: unsupported bpp: %d",
- __LINE__, (tui8)sz);
+ __LINE__, (tui8)bpp);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
if (s_check_rem(c->in_s, 2))
{
/* reading domain */
- in_uint16_be(c->in_s, sz);
-
- if (sz > 0)
+ if (!in_string16(c->in_s, buf, "domain", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+ if (buf[0] != '\0')
{
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
scp_session_set_domain(session, buf);
- g_free(buf);
}
}
if (s_check_rem(c->in_s, 2))
{
/* reading program */
- in_uint16_be(c->in_s, sz);
+ if (!in_string16(c->in_s, buf, "program", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
- if (sz > 0)
+ if (buf[0] != '\0')
{
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
scp_session_set_program(session, buf);
- g_free(buf);
}
}
if (s_check_rem(c->in_s, 2))
{
/* reading directory */
- in_uint16_be(c->in_s, sz);
+ if (!in_string16(c->in_s, buf, "directory", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
- if (sz > 0)
+ if (buf[0] != '\0')
{
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
scp_session_set_directory(session, buf);
- g_free(buf);
}
}
if (s_check_rem(c->in_s, 2))
{
/* reading client IP address */
- in_uint16_be(c->in_s, sz);
-
- if (sz > 0)
+ if (!in_string16(c->in_s, buf, "client IP", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+ if (buf[0] != '\0')
{
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
scp_session_set_client_ip(session, buf);
- g_free(buf);
}
}
}
else if (code == SCP_GW_AUTHENTICATION)
{
- /* g_writeln("Command is SCP_GW_AUTHENTICATION"); */
- session = scp_session_create();
-
- if (0 == session)
- {
- /* until syslog merge log_message(s_log, LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__);*/
- return SCP_SERVER_STATE_INTERNAL_ERR;
- }
-
- scp_session_set_version(session, version);
scp_session_set_type(session, SCP_GW_AUTHENTICATION);
/* reading username */
- in_uint16_be(c->in_s, sz);
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
+ if (!in_string16(c->in_s, buf, "username", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
/* g_writeln("Received user name: %s",buf); */
if (0 != scp_session_set_username(session, buf))
{
- scp_session_destroy(session);
/* until syslog merge log_message(s_log, LOG_LEVEL_WARNING, "[v0:%d] connection aborted: error setting username", __LINE__);*/
- g_free(buf);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- g_free(buf);
/* reading password */
- in_uint16_be(c->in_s, sz);
- buf = g_new0(char, sz + 1);
- in_uint8a(c->in_s, buf, sz);
- buf[sz] = '\0';
+ if (!in_string16(c->in_s, buf, "passwd", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
/* g_writeln("Received password: %s",buf); */
if (0 != scp_session_set_password(session, buf))
{
- scp_session_destroy(session);
/* until syslog merge log_message(s_log, LOG_LEVEL_WARNING, "[v0:%d] connection aborted: error setting password", __LINE__); */
- g_free(buf);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- g_free(buf);
}
else
{
@@ -380,10 +418,67 @@ scp_v0s_accept(struct SCP_CONNECTION *c,
return SCP_SERVER_STATE_SEQUENCE_ERR;
}
- (*s) = session;
return SCP_SERVER_STATE_OK;
}
+
+/* server API */
+/******************************************************************************/
+enum SCP_SERVER_STATES_E
+scp_v0s_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s, int skipVchk)
+{
+ enum SCP_SERVER_STATES_E result = SCP_SERVER_STATE_OK;
+ struct SCP_SESSION *session = NULL;
+ tui32 version = 0;
+
+ if (!skipVchk)
+ {
+ LOG_DBG("[v0:%d] starting connection", __LINE__);
+
+ if (0 == scp_tcp_force_recv(c->in_sck, c->in_s->data, 8))
+ {
+ c->in_s->end = c->in_s->data + 8;
+ in_uint32_be(c->in_s, version);
+
+ if (version != 0)
+ {
+ log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: version error", __LINE__);
+ result = SCP_SERVER_STATE_VERSION_ERR;
+ }
+ }
+ else
+ {
+ log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__);
+ result = SCP_SERVER_STATE_NETWORK_ERR;
+ }
+ }
+
+ if (result == SCP_SERVER_STATE_OK)
+ {
+ session = scp_session_create();
+ if (NULL == session)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v0:%d] connection aborted: no memory",
+ __LINE__);
+ result = SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+ else
+ {
+ result = scp_v0s_init_session(c, session);
+ if (result != SCP_SERVER_STATE_OK)
+ {
+ scp_session_destroy(session);
+ session = NULL;
+ }
+ }
+ }
+
+ (*s) = session;
+
+ return result;
+}
+
/******************************************************************************/
enum SCP_SERVER_STATES_E
scp_v0s_allow_connection(struct SCP_CONNECTION *c, SCP_DISPLAY d, const tui8 *guid)
Index: xrdp-0.9.6/sesman/libscp/libscp_v1s.c
===================================================================
--- xrdp-0.9.6.orig/sesman/libscp/libscp_v1s.c
+++ xrdp-0.9.6/sesman/libscp/libscp_v1s.c
@@ -35,16 +35,194 @@
//extern struct log_config* s_log;
+/**
+ * Reads a uint8 followed by a string into a buffer
+ *
+ * Buffer is null-terminated on success
+ *
+ * @param s Input stream
+ * @param [out] Output buffer (must be >= 256 chars)
+ * @param param Parameter we're reading
+ * @param line Line number reference
+ * @return != 0 if string read OK
+ *
+ * @todo
+ * This needs to be merged with the func of the same name in
+ * libscp_v1s_mng.c
+ */
+static
+int in_string8(struct stream *s, char str[], const char *param, int line)
+{
+ int result;
+
+ if (!s_check_rem(s, 1))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: %s len missing",
+ line, param);
+ result = 0;
+ }
+ else
+ {
+ unsigned int sz;
+
+ in_uint8(s, sz);
+ result = s_check_rem(s, sz);
+ if (!result)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: %s data missing",
+ line, param);
+ }
+ else
+ {
+ in_uint8a(s, str, sz);
+ str[sz] = '\0';
+ }
+ }
+ return result;
+}
+/* server API */
+
+/**
+ * Initialises a V1 session object
+ *
+ * This is called after the V1 header, command set and command have been read
+ *
+ * @param c Connection
+ * @param [out] session pre-allocated session object
+ * @return SCP_SERVER_STATE_OK for success
+ */
+static enum SCP_SERVER_STATES_E
+scp_v1s_init_session(struct SCP_CONNECTION *c, struct SCP_SESSION *session)
+{
+ tui8 type;
+ tui16 height;
+ tui16 width;
+ tui8 bpp;
+ tui8 sz;
+ char buf[256];
+
+ scp_session_set_version(session, 1);
+
+ /* Check there's data for the session type, the height, the width, the
+ * bpp, the resource sharing indicator and the locale */
+ if (!s_check_rem(c->in_s, 1 + 2 + 2 + 1 + 1 + 17))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: short packet",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ in_uint8(c->in_s, type);
+
+ if ((type != SCP_SESSION_TYPE_XVNC) && (type != SCP_SESSION_TYPE_XRDP))
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: unknown session type", __LINE__);
+ return SCP_SERVER_STATE_SESSION_TYPE_ERR;
+ }
+
+ scp_session_set_type(session, type);
+
+ in_uint16_be(c->in_s, height);
+ scp_session_set_height(session, height);
+ in_uint16_be(c->in_s, width);
+ scp_session_set_width(session, width);
+ in_uint8(c->in_s, bpp);
+ if (0 != scp_session_set_bpp(session, bpp))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: unsupported bpp: %d",
+ __LINE__, bpp);
+ return SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+ in_uint8(c->in_s, sz);
+ scp_session_set_rsr(session, sz);
+ in_uint8a(c->in_s, buf, 17);
+ buf[17] = '\0';
+ scp_session_set_locale(session, buf);
+
+ /* Check there's enough data left for at least an IPv4 address (+len) */
+ if (!s_check_rem(c->in_s, 1 + 4))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: IP addr len missing",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ in_uint8(c->in_s, sz);
+
+ if (sz == SCP_ADDRESS_TYPE_IPV4)
+ {
+ tui32 ipv4;
+ in_uint32_be(c->in_s, ipv4);
+ scp_session_set_addr(session, sz, &ipv4);
+ }
+ else if (sz == SCP_ADDRESS_TYPE_IPV6)
+ {
+ if (!s_check_rem(c->in_s, 16))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: IP addr missing",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+ in_uint8a(c->in_s, buf, 16);
+ scp_session_set_addr(session, sz, buf);
+ }
+
+ /* reading hostname */
+ if (!in_string8(c->in_s, buf, "hostname", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ if (0 != scp_session_set_hostname(session, buf))
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
+ return SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+
+ /* reading username */
+ if (!in_string8(c->in_s, buf, "username", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ if (0 != scp_session_set_username(session, buf))
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
+ return SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+
+ /* reading password */
+ if (!in_string8(c->in_s, buf, "passwd", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ if (0 != scp_session_set_password(session, buf))
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
+ return SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+
+ return SCP_SERVER_STATE_OK;
+}
+
/* server API */
enum SCP_SERVER_STATES_E scp_v1s_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s, int skipVchk)
{
+ enum SCP_SERVER_STATES_E result;
struct SCP_SESSION *session;
tui32 version;
tui32 size;
tui16 cmdset;
tui16 cmd;
- tui8 sz;
- char buf[257];
+
+ (*s) = NULL;
if (!skipVchk)
{
@@ -68,13 +246,15 @@ enum SCP_SERVER_STATES_E scp_v1s_accept(
in_uint32_be(c->in_s, size);
- if (size < 12)
+ /* Check the message is big enough for the header, the command set, and
+ * the command (but not too big) */
+ if (size < (8 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
{
log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: size error", __LINE__);
return SCP_SERVER_STATE_SIZE_ERR;
}
- init_stream(c->in_s, c->in_s->size);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, (size - 8)))
{
@@ -82,6 +262,8 @@ enum SCP_SERVER_STATES_E scp_v1s_accept(
return SCP_SERVER_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
/* reading command set */
in_uint16_be(c->in_s, cmdset);
@@ -111,98 +293,27 @@ enum SCP_SERVER_STATES_E scp_v1s_accept(
session = scp_session_create();
- if (0 == session)
- {
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error (malloc returned NULL)", __LINE__);
- return SCP_SERVER_STATE_INTERNAL_ERR;
- }
-
- scp_session_set_version(session, 1);
-
- in_uint8(c->in_s, sz);
-
- if ((sz != SCP_SESSION_TYPE_XVNC) && (sz != SCP_SESSION_TYPE_XRDP))
- {
- scp_session_destroy(session);
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: unknown session type", __LINE__);
- return SCP_SERVER_STATE_SESSION_TYPE_ERR;
- }
-
- scp_session_set_type(session, sz);
-
- in_uint16_be(c->in_s, cmd);
- scp_session_set_height(session, cmd);
- in_uint16_be(c->in_s, cmd);
- scp_session_set_height(session, cmd);
- in_uint8(c->in_s, sz);
- if (0 != scp_session_set_bpp(session, sz))
+ if (NULL == session)
{
- scp_session_destroy(session);
log_message(LOG_LEVEL_WARNING,
- "[v1s:%d] connection aborted: unsupported bpp: %d",
- __LINE__, sz);
- return SCP_SERVER_STATE_INTERNAL_ERR;
- }
- in_uint8(c->in_s, sz);
- scp_session_set_rsr(session, sz);
- in_uint8a(c->in_s, buf, 17);
- buf[17] = '\0';
- scp_session_set_locale(session, buf);
-
- in_uint8(c->in_s, sz);
-
- if (sz == SCP_ADDRESS_TYPE_IPV4)
- {
- in_uint32_be(c->in_s, size);
- scp_session_set_addr(session, sz, &size);
- }
- else if (sz == SCP_ADDRESS_TYPE_IPV6)
- {
- in_uint8a(c->in_s, buf, 16);
- scp_session_set_addr(session, sz, buf);
- }
-
- buf[256] = '\0';
- /* reading hostname */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
-
- if (0 != scp_session_set_hostname(session, buf))
- {
- scp_session_destroy(session);
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
- return SCP_SERVER_STATE_INTERNAL_ERR;
+ "[v1s:%d] connection aborted: internal error "
+ "(malloc returned NULL)", __LINE__);
+ result = SCP_SERVER_STATE_INTERNAL_ERR;
}
-
- /* reading username */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
-
- if (0 != scp_session_set_username(session, buf))
+ else
{
- scp_session_destroy(session);
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
- return SCP_SERVER_STATE_INTERNAL_ERR;
- }
-
- /* reading password */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
-
- if (0 != scp_session_set_password(session, buf))
- {
- scp_session_destroy(session);
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: internal error", __LINE__);
- return SCP_SERVER_STATE_INTERNAL_ERR;
+ result = scp_v1s_init_session(c, session);
+ if (result != SCP_SERVER_STATE_OK)
+ {
+ scp_session_destroy(session);
+ session = NULL;
+ }
}
/* returning the struct */
(*s) = session;
- return SCP_SERVER_STATE_OK;
+ return result;
}
enum SCP_SERVER_STATES_E
@@ -242,13 +353,12 @@ enum SCP_SERVER_STATES_E
scp_v1s_request_password(struct SCP_CONNECTION *c, struct SCP_SESSION *s,
const char *reason)
{
- tui8 sz;
tui32 version;
tui32 size;
tui16 cmdset;
tui16 cmd;
int rlen;
- char buf[257];
+ char buf[256];
init_stream(c->in_s, c->in_s->size);
init_stream(c->out_s, c->out_s->size);
@@ -296,13 +406,17 @@ scp_v1s_request_password(struct SCP_CONN
in_uint32_be(c->in_s, size);
- if (size < 12)
+ /* Check the message is big enough for the header, the command set, and
+ * the command (but not too big) */
+ if (size < (8 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
{
- log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: size error", __LINE__);
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s:%d] connection aborted: size error",
+ __LINE__);
return SCP_SERVER_STATE_SIZE_ERR;
}
- init_stream(c->in_s, c->in_s->size);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, (size - 8)))
{
@@ -310,6 +424,8 @@ scp_v1s_request_password(struct SCP_CONN
return SCP_SERVER_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
in_uint16_be(c->in_s, cmdset);
if (cmdset != SCP_COMMAND_SET_DEFAULT)
@@ -326,11 +442,11 @@ scp_v1s_request_password(struct SCP_CONN
return SCP_SERVER_STATE_SEQUENCE_ERR;
}
- buf[256] = '\0';
/* reading username */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
+ if (!in_string8(c->in_s, buf, "username", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_username(s, buf))
{
@@ -339,9 +455,10 @@ scp_v1s_request_password(struct SCP_CONN
}
/* reading password */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
+ if (!in_string8(c->in_s, buf, "passwd", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_password(s, buf))
{
@@ -468,13 +585,15 @@ scp_v1s_list_sessions(struct SCP_CONNECT
in_uint32_be(c->in_s, size);
- if (size < 12)
+ /* Check the message is big enough for the header, the command set, and
+ * the command (but not too big) */
+ if (size < (8 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
{
log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: size error", __LINE__);
return SCP_SERVER_STATE_SIZE_ERR;
}
- init_stream(c->in_s, c->in_s->size);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, (size - 8)))
{
@@ -482,6 +601,8 @@ scp_v1s_list_sessions(struct SCP_CONNECT
return SCP_SERVER_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
in_uint16_be(c->in_s, cmd);
if (cmd != SCP_COMMAND_SET_DEFAULT)
@@ -606,14 +727,16 @@ scp_v1s_list_sessions(struct SCP_CONNECT
in_uint32_be(c->in_s, size);
- if (size < 12)
+ /* Check the message is big enough for the header, the command set, and
+ * the command (but not too big) */
+ if (size < (8 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
{
log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: size error", __LINE__);
return SCP_SERVER_STATE_SIZE_ERR;
}
/* rest of the packet */
- init_stream(c->in_s, c->in_s->size);
+ init_stream(c->in_s, size - 8);
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, (size - 8)))
{
@@ -621,6 +744,8 @@ scp_v1s_list_sessions(struct SCP_CONNECT
return SCP_SERVER_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
in_uint16_be(c->in_s, cmd);
if (cmd != SCP_COMMAND_SET_DEFAULT)
@@ -633,6 +758,11 @@ scp_v1s_list_sessions(struct SCP_CONNECT
if (cmd == 43)
{
+ if (!s_check_rem(c->in_s, 4))
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: missing session", __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
/* select session */
in_uint32_be(c->in_s, (*sid));
Index: xrdp-0.9.6/sesman/libscp/libscp_v1s_mng.c
===================================================================
--- xrdp-0.9.6.orig/sesman/libscp/libscp_v1s_mng.c
+++ xrdp-0.9.6/sesman/libscp/libscp_v1s_mng.c
@@ -38,17 +38,79 @@
static enum SCP_SERVER_STATES_E
_scp_v1s_mng_check_response(struct SCP_CONNECTION *c, struct SCP_SESSION *s);
-/* server API */
-enum SCP_SERVER_STATES_E
-scp_v1s_mng_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s)
+/**
+ * Reads a uint8 followed by a string into a buffer
+ *
+ * Buffer is null-terminated on success
+ *
+ * @param s Input stream
+ * @param [out] Output buffer (must be >= 256 chars)
+ * @param param Parameter we're reading
+ * @param line Line number reference
+ * @return != 0 if string read OK
+ *
+ * @todo
+ * This needs to be merged with the func of the same name in
+ * libscp_v1s.c
+ */
+static
+int in_string8(struct stream *s, char str[], const char *param, int line)
+{
+ int result;
+
+ if (!s_check_rem(s, 1))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s_mng:%d] connection aborted: %s len missing",
+ line, param);
+ result = 0;
+ }
+ else
+ {
+ unsigned int sz;
+
+ in_uint8(s, sz);
+ result = s_check_rem(s, sz);
+ if (!result)
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s_mng:%d] connection aborted: %s data missing",
+ line, param);
+ }
+ else
+ {
+ in_uint8a(s, str, sz);
+ str[sz] = '\0';
+ }
+ }
+ return result;
+}
+/**
+ * Initialises a V1 management session object
+ *
+ * At call time, the command set value has been read from the wire, and
+ * the command still needs to be processed.
+ *
+ * @param c Connection
+ * @param [out] session pre-allocated session object
+ * @return SCP_SERVER_STATE_START_MANAGE for success
+ */
+static enum SCP_SERVER_STATES_E
+scp_v1s_mng_init_session(struct SCP_CONNECTION *c, struct SCP_SESSION *session)
{
- struct SCP_SESSION *session;
tui32 ipaddr;
tui16 cmd;
tui8 sz;
- char buf[257];
+ char buf[256];
+
+ scp_session_set_version(session, 1);
/* reading command */
+ if (!s_check_rem(c->in_s, 2))
+ {
+ /* Caller should have checked this */
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
in_uint16_be(c->in_s, cmd);
if (cmd != 1) /* manager login */
@@ -56,41 +118,39 @@ scp_v1s_mng_accept(struct SCP_CONNECTION
return SCP_SERVER_STATE_SEQUENCE_ERR;
}
- session = scp_session_create();
-
- if (0 == session)
+ /* reading username */
+ if (!in_string8(c->in_s, buf, "username", __LINE__))
{
- return SCP_SERVER_STATE_INTERNAL_ERR;
+ return SCP_SERVER_STATE_SIZE_ERR;
}
- scp_session_set_version(session, 1);
- scp_session_set_type(session, SCP_SESSION_TYPE_MANAGE);
-
- /* reading username */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
-
if (0 != scp_session_set_username(session, buf))
{
- scp_session_destroy(session);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
/* reading password */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
+ if (!in_string8(c->in_s, buf, "passwd", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_password(session, buf))
{
- scp_session_destroy(session);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- /* reading remote address */
- in_uint8(c->in_s, sz);
+ /* reading remote address
+ * Check there's enough data left for at least an IPv4 address (+len) */
+ if (!s_check_rem(c->in_s, 1 + 4))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s_mng:%d] connection aborted: IP addr len missing",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+ in_uint8(c->in_s, sz);
if (sz == SCP_ADDRESS_TYPE_IPV4)
{
in_uint32_be(c->in_s, ipaddr);
@@ -98,25 +158,57 @@ scp_v1s_mng_accept(struct SCP_CONNECTION
}
else if (sz == SCP_ADDRESS_TYPE_IPV6)
{
+ if (!s_check_rem(c->in_s, 16))
+ {
+ log_message(LOG_LEVEL_WARNING,
+ "[v1s_mng:%d] connection aborted: IP addr missing",
+ __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
in_uint8a(c->in_s, buf, 16);
scp_session_set_addr(session, sz, buf);
}
/* reading hostname */
- in_uint8(c->in_s, sz);
- buf[sz] = '\0';
- in_uint8a(c->in_s, buf, sz);
+ if (!in_string8(c->in_s, buf, "hostname", __LINE__))
+ {
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
if (0 != scp_session_set_hostname(session, buf))
{
- scp_session_destroy(session);
return SCP_SERVER_STATE_INTERNAL_ERR;
}
- /* returning the struct */
+ return SCP_SERVER_STATE_START_MANAGE;
+}
+
+enum SCP_SERVER_STATES_E
+scp_v1s_mng_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s)
+{
+ enum SCP_SERVER_STATES_E result;
+ struct SCP_SESSION *session;
+
+ session = scp_session_create();
+ if (NULL == session)
+ {
+ result = SCP_SERVER_STATE_INTERNAL_ERR;
+ }
+ else
+ {
+ scp_session_set_type(session, SCP_SESSION_TYPE_MANAGE);
+
+ result = scp_v1s_mng_init_session(c, session);
+ if (result != SCP_SERVER_STATE_START_MANAGE)
+ {
+ scp_session_destroy(session);
+ session = NULL;
+ }
+ }
+
(*s) = session;
- return SCP_SERVER_STATE_START_MANAGE;
+ return result;
}
/* 002 */
@@ -312,7 +404,15 @@ _scp_v1s_mng_check_response(struct SCP_C
in_uint32_be(c->in_s, size);
- init_stream(c->in_s, c->in_s->size);
+ /* Check the message is big enough for the header, the command set, and
+ * the command (but not too big) */
+ if (size < (8 + 2 + 2) || size > SCP_MAX_MESSAGE_SIZE)
+ {
+ log_message(LOG_LEVEL_WARNING, "[v1s_mng:%d] connection aborted: size error", __LINE__);
+ return SCP_SERVER_STATE_SIZE_ERR;
+ }
+
+ init_stream(c->in_s, size - 8);
/* read the rest of the packet */
if (0 != scp_tcp_force_recv(c->in_sck, c->in_s->data, size - 8))
@@ -321,6 +421,8 @@ _scp_v1s_mng_check_response(struct SCP_C
return SCP_SERVER_STATE_NETWORK_ERR;
}
+ c->in_s->end = c->in_s->data + (size - 8);
+
in_uint16_be(c->in_s, cmd);
if (cmd != SCP_COMMAND_SET_MANAGE)
Index: xrdp-0.9.6/sesman/scp.c
===================================================================
--- xrdp-0.9.6.orig/sesman/scp.c
+++ xrdp-0.9.6/sesman/scp.c
@@ -48,8 +48,8 @@ scp_process_start(void *sck)
make_stream(scon.in_s);
make_stream(scon.out_s);
- init_stream(scon.in_s, 8192);
- init_stream(scon.out_s, 8192);
+ init_stream(scon.in_s, SCP_MAX_MESSAGE_SIZE);
+ init_stream(scon.out_s, SCP_MAX_MESSAGE_SIZE);
switch (scp_vXs_accept(&scon, &(sdata)))
{
@@ -76,10 +76,12 @@ scp_process_start(void *sck)
scp_v1_mng_process(&scon, sdata);
break;
case SCP_SERVER_STATE_VERSION_ERR:
- /* an unknown scp version was requested, so we shut down the */
- /* connection (and log the fact) */
+ case SCP_SERVER_STATE_SIZE_ERR:
+ /* an unknown scp version was requested, or the message sizes
+ are inconsistent. Shut down the connection and log the
+ fact */
log_message(LOG_LEVEL_WARNING,
- "unknown protocol version specified. connection refused.");
+ "protocol violation. connection refused.");
break;
case SCP_SERVER_STATE_NETWORK_ERR:
log_message(LOG_LEVEL_WARNING, "libscp network error.");
