Unrated severityNVD Advisory· Published Jun 30, 2020· Updated Aug 4, 2024
Local users can perform a buffer overflow attack against the xrdp-sesman service and then impersonate it
CVE-2020-4044
Description
The xrdp-sesman service before version 0.9.13.1 can be crashed by connecting over port 3350 and supplying a malicious payload. Once the xrdp-sesman process is dead, an unprivileged attacker on the server could then proceed to start their own imposter sesman service listening on port 3350. This will allow them to capture any user credentials that are submitted to XRDP and approve or reject arbitrary login credentials. For xorgxrdp sessions in particular, this allows an unauthorized user to hijack an existing session. This is a buffer overflow attack, so there may be a risk of arbitrary code execution as well.
Affected products
1- Range: < 0.9.13.1
Patches
2a9d4130d20c1Merge pull request #1624 from metalefty/v0.9-CVE-2020-4044
8 files changed · +615 −268
configure.ac+1 −1 modified@@ -1,7 +1,7 @@ # Process this file with autoconf to produce a configure script AC_PREREQ(2.65) -AC_INIT([xrdp], [0.9.13], [xrdp-devel@googlegroups.com]) +AC_INIT([xrdp], [0.9.13.1], [xrdp-devel@googlegroups.com]) AC_CONFIG_HEADERS(config_ac.h:config_ac-h.in) AM_INIT_AUTOMAKE([1.7.2 foreign]) AC_CONFIG_MACRO_DIR([m4])
NEWS.md+15 −1 modified@@ -1,3 +1,17 @@ +# Release notes for xrdp v0.9.13.1 (2020/06/30) + +This is a security fix release that includes fixes for the following local buffer overflow vulnerability. + +* [CVE-2022-4044: Local users can perform a buffer overflow attack against the xrdp-sesman service and then impersonate it](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-4044) + +This update is recommended for all xrdp users. + +## Special thanks + +Thanks to [Ashley Newson](https://github.com/ashleynewson) reporting the vulnerability and reviewing fix. + +----------------------- + # Release notes for xrdp v0.9.13 (2020/03/11) This release is an intermediate bugfix release. The previous version v0.9.12 has some regressions on drive redirection. @@ -111,7 +125,7 @@ Thank you for matt335672 contributing to lots of improvements in drive redirecti ----------------------- -## Release notes for xrdp v0.9.9 (2018/12/25) +# Release notes for xrdp v0.9.9 (2018/12/25) ## Release cycle From the next release, release cycle will be changed from quarterly to every
README.md+1 −1 modified@@ -2,7 +2,7 @@ [](https://gitter.im/neutrinolabs/xrdp-questions)  -*Current Version:* 0.9.13 +*Current Version:* 0.9.13.1 # xrdp - an open source RDP server
sesman/libscp/libscp_types.h+4 −0 modified@@ -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;
sesman/libscp/libscp_v0.c+217 −122 modified@@ -34,13 +34,72 @@ 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 scp_v0c_connect(struct SCP_CONNECTION *c, struct SCP_SESSION *s) { tui32 version; - tui32 size; + int size; tui16 sz; init_stream(c->in_s, c->in_s->size); @@ -71,10 +130,24 @@ scp_v0c_connect(struct SCP_CONNECTION *c, struct SCP_SESSION *s) } 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,21 +184,25 @@ scp_v0c_connect(struct SCP_CONNECTION *c, struct SCP_SESSION *s) 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 = %d", + __LINE__, 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)) { log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__); 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, struct SCP_SESSION *s) 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; + int size; + tui16 height; + tui16 width; + tui16 bpp; tui32 code = 0; - char *buf = 0; - - if (!skipVchk) - { - LOG_DBG("[v0:%d] starting connection", __LINE__); + char buf[STRING16_MAX_LEN + 1]; - 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 = %d", + __LINE__, 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, struct SCP_SESSION **s, int skipVchk) 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,165 +287,198 @@ scp_v0s_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s, int skipVchk) } /* 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 { log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: sequence error", __LINE__); 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)
sesman/libscp/libscp_v1s.c+236 −106 modified@@ -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; + int size; tui16 cmdset; tui16 cmd; - tui8 sz; - char buf[257]; + + (*s) = NULL; if (!skipVchk) { @@ -68,20 +246,24 @@ enum SCP_SERVER_STATES_E scp_v1s_accept(struct SCP_CONNECTION *c, struct SCP_SES 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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(struct SCP_CONNECTION *c, struct SCP_SES 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)) + if (NULL == session) { - 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_width(session, cmd); - in_uint8(c->in_s, sz); - if (0 != scp_session_set_bpp(session, sz)) - { - 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); + "[v1s:%d] connection aborted: internal error " + "(malloc returned NULL)", __LINE__); + result = SCP_SERVER_STATE_INTERNAL_ERR; } - - 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; - } - - /* 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); - 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)) + else { - 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; + int 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,20 +406,26 @@ scp_v1s_request_password(struct SCP_CONNECTION *c, struct SCP_SESSION *s, 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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_CONNECTION *c, struct SCP_SESSION *s, 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_CONNECTION *c, struct SCP_SESSION *s, } /* 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)) { @@ -422,7 +539,7 @@ enum SCP_SERVER_STATES_E scp_v1s_list_sessions(struct SCP_CONNECTION *c, int sescnt, struct SCP_DISCONNECTED_SESSION *ds, SCP_SID *sid) { tui32 version = 1; - tui32 size = 12; + int size = 12; tui16 cmd = 40; int pktcnt; int idx; @@ -468,20 +585,24 @@ scp_v1s_list_sessions(struct SCP_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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,21 +727,25 @@ scp_v1s_list_sessions(struct SCP_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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));
sesman/libscp/libscp_v1s_mng.c+134 −32 modified@@ -38,85 +38,177 @@ 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 */ { 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); scp_session_set_addr(session, sz, &ipaddr); } 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 */ @@ -289,7 +381,7 @@ static enum SCP_SERVER_STATES_E _scp_v1s_mng_check_response(struct SCP_CONNECTION *c, struct SCP_SESSION *s) { tui32 version; - tui32 size; + int size; tui16 cmd; // tui8 dim; // char buf[257]; @@ -312,7 +404,15 @@ _scp_v1s_mng_check_response(struct SCP_CONNECTION *c, struct SCP_SESSION *s) 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_CONNECTION *c, struct SCP_SESSION *s) 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)
sesman/scp.c+7 −5 modified@@ -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.");
0c791d073d0eMerge pull request from GHSA-j9fv-6fwf-p3g4
5 files changed · +592 −259
sesman/libscp/libscp_types.h+4 −0 modified@@ -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;
sesman/libscp/libscp_v0.c+215 −120 modified@@ -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, struct SCP_SESSION *s) } 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,21 +184,25 @@ scp_v0c_connect(struct SCP_CONNECTION *c, struct SCP_SESSION *s) 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)) { log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: network error", __LINE__); 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, struct SCP_SESSION *s) 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; - - if (!skipVchk) - { - LOG_DBG("[v0:%d] starting connection", __LINE__); + char buf[STRING16_MAX_LEN + 1]; - 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, struct SCP_SESSION **s, int skipVchk) 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,165 +287,198 @@ scp_v0s_accept(struct SCP_CONNECTION *c, struct SCP_SESSION **s, int skipVchk) } /* 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 { log_message(LOG_LEVEL_WARNING, "[v0:%d] connection aborted: sequence error", __LINE__); 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)
sesman/libscp/libscp_v1s.c+233 −103 modified@@ -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,20 +246,24 @@ enum SCP_SERVER_STATES_E scp_v1s_accept(struct SCP_CONNECTION *c, struct SCP_SES 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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(struct SCP_CONNECTION *c, struct SCP_SES 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)) + if (NULL == session) { - 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_width(session, cmd); - in_uint8(c->in_s, sz); - if (0 != scp_session_set_bpp(session, sz)) - { - 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); + "[v1s:%d] connection aborted: internal error " + "(malloc returned NULL)", __LINE__); + result = SCP_SERVER_STATE_INTERNAL_ERR; } - - 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; - } - - /* 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); - 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)) + else { - 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,20 +406,26 @@ scp_v1s_request_password(struct SCP_CONNECTION *c, struct SCP_SESSION *s, 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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_CONNECTION *c, struct SCP_SESSION *s, 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_CONNECTION *c, struct SCP_SESSION *s, } /* 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,20 +585,24 @@ scp_v1s_list_sessions(struct SCP_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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,21 +727,25 @@ scp_v1s_list_sessions(struct SCP_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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))) { log_message(LOG_LEVEL_WARNING, "[v1s:%d] connection aborted: network error", __LINE__); 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_CONNECTION *c, int sescnt, struct SCP_DISCONNEC 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));
sesman/libscp/libscp_v1s_mng.c+133 −31 modified@@ -38,85 +38,177 @@ 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 */ { 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); scp_session_set_addr(session, sz, &ipaddr); } 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_CONNECTION *c, struct SCP_SESSION *s) 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_CONNECTION *c, struct SCP_SESSION *s) 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)
sesman/scp.c+7 −5 modified@@ -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.");
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
7- lists.opensuse.org/opensuse-security-announce/2020-07/msg00036.htmlmitrevendor-advisoryx_refsource_SUSE
- lists.opensuse.org/opensuse-security-announce/2020-08/msg00037.htmlmitrevendor-advisoryx_refsource_SUSE
- www.debian.org/security/2020/dsa-4737mitrevendor-advisoryx_refsource_DEBIAN
- github.com/neutrinolabs/xrdp/commit/0c791d073d0eb344ee7aaafd221513dc9226762cmitrex_refsource_MISC
- github.com/neutrinolabs/xrdp/releases/tag/v0.9.13.1mitrex_refsource_MISC
- github.com/neutrinolabs/xrdp/security/advisories/GHSA-j9fv-6fwf-p3g4mitrex_refsource_CONFIRM
- lists.debian.org/debian-lts-announce/2020/08/msg00015.htmlmitremailing-listx_refsource_MLIST
News mentions
0No linked articles in our index yet.