Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP1:GA
xrdp
xrdp-cve-2020-4044-fix-0.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File xrdp-cve-2020-4044-fix-0.patch of Package xrdp
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.");
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
