diff --git a/common/log.h b/common/log.h index 2f8063e1..7545b1f0 100644 --- a/common/log.h +++ b/common/log.h @@ -114,7 +114,7 @@ internal_log_lvl2str(const enum logLevels lvl, char *str); * */ enum logLevels -internal_log_text2level(const char *s); +internal_log_text2level(const char *buf); /** * A function that init our struct that holds all state and diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index ad81d0df..03e03c39 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -74,8 +74,11 @@ #define CONNECTION_TYPE_LAN 0x06 #define CONNECTION_TYPE_AUTODETECT 0x07 -/* Virtual channel options */ -/* Channel Definition Structure: options (2.2.1.3.4.1) */ +/* Channel definition structure CHANNEL_DEF (2.2.1.3.4.1) */ +/* This isn't explicitly named in MS-RDPBCGR */ +#define CHANNEL_NAME_LEN 7 + +/* Oprions field */ /* NOTE: XR_ prefixed to avoid conflict with FreeRDP */ #define XR_CHANNEL_OPTION_INITIALIZED 0x80000000 #define XR_CHANNEL_OPTION_ENCRYPT_RDP 0x40000000 diff --git a/common/trans.c b/common/trans.c index 883d7cbd..27efa8f8 100644 --- a/common/trans.c +++ b/common/trans.c @@ -452,17 +452,14 @@ trans_force_read_s(struct trans *self, struct stream *in_s, int size) { int rcvd; - if (self->status != TRANS_STATUS_UP) + if (self->status != TRANS_STATUS_UP || + size < 0 || !s_check_rem_out(in_s, size)) { return 1; } + while (size > 0) { - /* make sure stream has room */ - if ((in_s->end + size) > (in_s->data + in_s->size)) - { - return 1; - } rcvd = self->trans_recv(self, in_s->end, size); if (rcvd == -1) { diff --git a/genkeymap/genkeymap.c b/genkeymap/genkeymap.c index 03e54d8c..3b654454 100644 --- a/genkeymap/genkeymap.c +++ b/genkeymap/genkeymap.c @@ -136,7 +136,7 @@ int main(int argc, char **argv) fprintf(outf, "[%s]\n", sections[idx]); e.state = states[idx]; - for (i = 8; i <= 137; i++) /* Keycodes */ + for (i = 8; i < 137; i++) /* Keycodes */ { if (is_evdev) e.keycode = xfree86_to_evdev[i-8]; diff --git a/instfiles/pam.d/mkpamrules b/instfiles/pam.d/mkpamrules index d90ad54b..a75bfc32 100755 --- a/instfiles/pam.d/mkpamrules +++ b/instfiles/pam.d/mkpamrules @@ -10,51 +10,87 @@ service="xrdp-sesman" pamdir="/etc/pam.d" pamdir_suse="/usr/etc/pam.d" +# Modules needed by xrdp-sesman.unix, if we get to that +unix_modules_needed="pam_unix.so pam_env.so pam_nologin.so" + +# Directories where pam modules might be installed +# Add to this list as platforms are added +pam_module_dir_searchpath="/lib*/security /usr/lib*/security /lib/*/security /usr/lib/*/security" + +find_pam_module_dir() +{ + # Looks for the pam security module directory + set -- $pam_module_dir_searchpath + for d in "$@"; do + if [ -s $d/pam_unix.so ]; then + echo $d + break + fi + done +} + +can_apply_unix_config() +{ + result=0 + module_dir="$1" + for m in $unix_modules_needed; do + if [ ! -s $module_dir/$m ]; then + echo " ** $m not found" >&2 + result=1 + fi + done + + return $result +} + guess_rules () { - if test -s "$pamdir/password-auth"; then + rules= + if [ -s "$pamdir/password-auth" ]; then rules="redhat" - return - fi - if test -s "$pamdir_suse/common-account"; then + elif [ -s "$pamdir_suse/common-account" ]; then rules="suse" - return - fi - if test -s "$pamdir/common-account"; then + elif [ -s "$pamdir/common-account" ]; then if grep "^@include" "$pamdir/passwd" >/dev/null 2>&1; then rules="debian" else rules="suse" fi - return - fi - if test ! -f "$pamdir/system-auth" -a -s "$pamdir/system"; then + elif [ ! -f "$pamdir/system-auth" -a -s "$pamdir/system" ]; then rules="freebsd" - return - fi - if test -s "$pamdir/authorization"; then + elif [ -s "$pamdir/authorization" ]; then rules="macos" - return - fi - if test -s "$pamdir/system-remote-login"; then + elif [ -s "$pamdir/system-remote-login" ]; then rules="arch" - return - fi - rules="unix" - return + elif [ -s "$pamdir/system-auth" ]; then + rules="system" + + else + module_dir=`find_pam_module_dir` + if [ -d "$module_dir" ]; then + #echo "- Found pam modules in $module_dir" >&2 + if can_apply_unix_config "$module_dir" ; then + rules="unix" + fi + fi + fi } -if test "$rules" = "auto"; then +if [ "$rules" = "auto" ]; then guess_rules + if [ -z "$rules" ]; then + echo "** Can't guess PAM rules for this system" + exit 1 + fi fi -if test -s "$srcdir/$service.$rules"; then +if [ -s "$srcdir/$service.$rules" ]; then ln -nsf "$srcdir/$service.$rules" "$outfile" else echo "Cannot find $srcdir/$service.$rules" diff --git a/instfiles/pam.d/xrdp-sesman.system b/instfiles/pam.d/xrdp-sesman.system new file mode 100644 index 00000000..5025a0f9 --- /dev/null +++ b/instfiles/pam.d/xrdp-sesman.system @@ -0,0 +1,5 @@ +#%PAM-1.0 +auth include system-auth +account include system-auth +password include system-auth +session include system-auth diff --git a/instfiles/pam.d/xrdp-sesman.unix b/instfiles/pam.d/xrdp-sesman.unix index 5025a0f9..3bee185c 100644 --- a/instfiles/pam.d/xrdp-sesman.unix +++ b/instfiles/pam.d/xrdp-sesman.unix @@ -1,5 +1,16 @@ #%PAM-1.0 -auth include system-auth -account include system-auth -password include system-auth -session include system-auth +# +# Really basic authentication set when nothing else is available +# +# You may need to edit this to suit your system depending on the +# required functionality. +# +auth required pam_unix.so shadow +auth required pam_env.so + +password required pam_unix.so + +account required pam_unix.so +account required pam_nologin.so + +session required pam_unix.so diff --git a/libxrdp/libxrdp.c b/libxrdp/libxrdp.c index 27e2c3b7..0160bcc8 100644 --- a/libxrdp/libxrdp.c +++ b/libxrdp/libxrdp.c @@ -125,24 +125,18 @@ libxrdp_force_read(struct trans* trans) if (trans_force_read(trans, 4) != 0) { - g_writeln("libxrdp_force_read: error"); + g_writeln("libxrdp_force_read: header read error"); return 0; } bytes = libxrdp_get_pdu_bytes(s->data); - if (bytes < 1) + if (bytes < 4 || bytes > s->size) { - g_writeln("libxrdp_force_read: error"); + g_writeln("libxrdp_force_read: bad header length %d", bytes); return 0; } - if (bytes > 32 * 1024) - { - g_writeln("libxrdp_force_read: error"); - return 0; - } - if (trans_force_read(trans, bytes - 4) != 0) { - g_writeln("libxrdp_force_read: error"); + g_writeln("libxrdp_force_read: Can't read PDU"); return 0; } return s; diff --git a/libxrdp/xrdp_iso.c b/libxrdp/xrdp_iso.c index a53af5ce..27d5a57a 100644 --- a/libxrdp/xrdp_iso.c +++ b/libxrdp/xrdp_iso.c @@ -129,6 +129,12 @@ xrdp_iso_process_rdp_neg_req(struct xrdp_iso *self, struct stream *s) int flags; int len; + if (!s_check_rem(s, 7)) + { + LLOGLN(10, ("xrdp_iso_process_rdpNegReq: unexpected end-of-record")); + return 1; + } + in_uint8(s, flags); if (flags != 0x0 && flags != 0x8 && flags != 0x1) { @@ -153,13 +159,22 @@ xrdp_iso_process_rdp_neg_req(struct xrdp_iso *self, struct stream *s) return 0; } -/*****************************************************************************/ -/* returns error */ +/***************************************************************************** + * Reads an X.224 PDU (X.224 section 13) preceded by a T.123 TPKT + * header (T.123 section 8) + * + * On entry, the TPKT header length field will have been inspected and used to + * set up the input stream. + * + * On exit, the TPKT header and the fixed part of the PDU header will have been + * removed from the stream. + * + * Returns error + *****************************************************************************/ static int xrdp_iso_recv_msg(struct xrdp_iso *self, struct stream *s, int *code, int *len) { int ver; - int plen; *code = 0; *len = 0; @@ -169,7 +184,16 @@ xrdp_iso_recv_msg(struct xrdp_iso *self, struct stream *s, int *code, int *len) LLOGLN(10, ("xrdp_iso_recv_msg error logic")); } + /* TPKT header is 4 bytes, then first 2 bytes of the X.224 CR-TPDU */ + if (!s_check_rem(s, 6)) + { + return 1; + } + in_uint8(s, ver); + in_uint8s(s, 3); /* Skip reserved field, plus length */ + in_uint8(s, *len); + in_uint8(s, *code); if (ver != 3) { @@ -178,24 +202,17 @@ xrdp_iso_recv_msg(struct xrdp_iso *self, struct stream *s, int *code, int *len) return 1; } - in_uint8s(s, 1); - in_uint16_be(s, plen); - - if (plen < 4) + if (*len == 255) { + /* X.224 13.2.1 - reserved value */ + LLOGLN(10, ("xrdp_iso_recv_msg: reserved length encountered")); + LHEXDUMP(10, (s->data, 4)); return 1; } - if (!s_check_rem(s, 2)) - { - return 1; - } - - in_uint8(s, *len); - in_uint8(s, *code); - if (*code == ISO_PDU_DT) { + /* Data PDU : X.224 13.7 */ if (!s_check_rem(s, 1)) { return 1; @@ -204,6 +221,7 @@ xrdp_iso_recv_msg(struct xrdp_iso *self, struct stream *s, int *code, int *len) } else { + /* Other supported PDUs : X.224 13.x */ if (!s_check_rem(s, 5)) { return 1; @@ -302,7 +320,20 @@ xrdp_iso_send_cc(struct xrdp_iso *self) free_stream(s); return 0; } -/*****************************************************************************/ +/***************************************************************************** + * Process an X.224 connection request PDU + * + * See MS-RDPCGR v20190923 sections 2.2.1.1 and 3.3.5.3.1. + * + * From the latter, in particular:- + * - The length embedded in the TPKT header MUST be examined for + * consistency with the received data. If there is a discrepancy, the + * connection SHOULD be dropped + * - If the optional routingToken field exists it MUST be ignored. + * - If the optional cookie field is present it MUST be ignored. + * - If both the routingToken and cookie fields are present, the server + * SHOULD continue with the connection. + *****************************************************************************/ /* returns error */ int xrdp_iso_incoming(struct xrdp_iso *self) @@ -310,16 +341,14 @@ xrdp_iso_incoming(struct xrdp_iso *self) int rv = 0; int code; int len; - int cookie_index; int cc_type; - char text[256]; - char *pend; struct stream *s; + int expected_pdu_len; LLOGLN(10, (" in xrdp_iso_incoming")); s = libxrdp_force_read(self->trans); - if (s == 0) + if (s == NULL) { return 1; } @@ -330,15 +359,29 @@ xrdp_iso_incoming(struct xrdp_iso *self) return 1; } - if ((code != ISO_PDU_CR) || (len < 6)) + if (code != ISO_PDU_CR) { return 1; } + /* + * Make sure the length indicator field extracted from the X.224 + * connection request TPDU corresponds to the length in the TPKT header. + * + * We do this by seeing how the indicator field minus the counted + * octets in the TPDU header (6) compares with the space left in + * the stream. + */ + expected_pdu_len = (s->end - s->p) + 6; + if (len != expected_pdu_len) + { + LLOGLN(0, ("xrdp_iso_incoming: X.224 CR-TPDU length exp %d got %d", + expected_pdu_len, len)); + return 1; + } + /* process connection request */ - pend = s->p + (len - 6); - cookie_index = 0; - while (s->p < pend) + while (s_check_rem(s, 1)) { in_uint8(s, cc_type); switch (cc_type) @@ -355,28 +398,30 @@ xrdp_iso_incoming(struct xrdp_iso *self) break; case RDP_CORRELATION_INFO: /* rdpCorrelationInfo 6 */ // TODO + if (!s_check_rem(s, 1 + 2 + 16 + 16)) + { + LLOGLN(0, ("xrdp_iso_incoming: short correlation info")); + return 1; + } + in_uint8s(s, 1 + 2 + 16 + 16); break; - case 'C': /* Cookie routingToken */ - while (s->p < pend) + case 'C': /* Cookie */ + /* The routingToken and cookie fields are both ASCII + * strings starting with the word 'Cookie: ' and + * ending with CR+LF. We ignore both, so we do + * not need to distinguish them */ + while (s_check_rem(s, 1)) { - text[cookie_index] = cc_type; - cookie_index++; - if (cookie_index > 255) - { - cookie_index = 255; - } - if ((s->p[0] == 0x0D) && (s->p[1] == 0x0A)) - { - in_uint8s(s, 2); - text[cookie_index] = 0; - cookie_index = 0; - if (g_strlen(text) > 0) - { - } - break; - } in_uint8(s, cc_type); + if (cc_type == 0x0D && s_check_rem(s, 1)) + { + in_uint8(s, cc_type); + if (cc_type == 0x0A) + { + break; + } + } } break; } diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index 6b1b9326..d7ef1869 100644 --- a/sesman/chansrv/chansrv.c +++ b/sesman/chansrv/chansrv.c @@ -39,6 +39,10 @@ #include "xrdp_sockets.h" #include "audin.h" +#include "ms-rdpbcgr.h" + +#define MAX_PATH 260 + static struct trans *g_lis_trans = 0; static struct trans *g_con_trans = 0; static struct trans *g_api_lis_trans = 0; @@ -1042,9 +1046,13 @@ my_api_trans_data_in(struct trans *trans) int rv; int bytes; int ver; - int channel_name_bytes; struct chansrv_drdynvc_procs procs; - char *chan_name; + /* + * Name is limited to CHANNEL_NAME_LEN for an SVC, or MAX_PATH + * bytes for a DVC + */ + char chan_name[MAX(CHANNEL_NAME_LEN, MAX_PATH) + 1]; + unsigned int channel_name_bytes; //g_writeln("my_api_trans_data_in: extra_flags %d", trans->extra_flags); rv = 0; @@ -1067,12 +1075,13 @@ my_api_trans_data_in(struct trans *trans) rv = 1; in_uint32_le(s, channel_name_bytes); //g_writeln("my_api_trans_data_in: channel_name_bytes %d", channel_name_bytes); - chan_name = g_new0(char, channel_name_bytes + 1); - if (chan_name == NULL) + if (channel_name_bytes > (sizeof(chan_name) - 1)) { return 1; } in_uint8a(s, chan_name, channel_name_bytes); + chan_name[channel_name_bytes] = '\0'; + in_uint32_le(s, ad->chan_flags); //g_writeln("my_api_trans_data_in: chan_name %s chan_flags 0x%8.8x", chan_name, ad->chan_flags); if (ad->chan_flags == 0) @@ -1132,7 +1141,6 @@ my_api_trans_data_in(struct trans *trans) // "chan_id %d", rv, ad->chan_id); g_drdynvcs[ad->chan_id].xrdp_api_trans = trans; } - g_free(chan_name); init_stream(s, 0); trans->extra_flags = 2; trans->header_size = 0; diff --git a/sesman/chansrv/smartcard_pcsc.c b/sesman/chansrv/smartcard_pcsc.c index 51409567..5f4516db 100644 --- a/sesman/chansrv/smartcard_pcsc.c +++ b/sesman/chansrv/smartcard_pcsc.c @@ -586,9 +586,18 @@ int scard_process_list_readers(struct trans *con, struct stream *in_s) { int hContext; - int bytes_groups; + unsigned int bytes_groups; int cchReaders; - char *groups; + /* + * At the time of writing, the groups strings which can be sent + * over this interface are all small:- + * + * "SCard$AllReaders", "SCard$DefaultReaders", "SCard$LocalReaders" and + * "SCard$SystemReaders" + * + * We'll allow a bit extra in case the interface changes + */ + char groups[256]; struct pcsc_uds_client *uds_client; struct pcsc_context *lcontext; struct pcsc_list_readers *pcscListReaders; @@ -597,8 +606,14 @@ scard_process_list_readers(struct trans *con, struct stream *in_s) uds_client = (struct pcsc_uds_client *) (con->callback_data); in_uint32_le(in_s, hContext); in_uint32_le(in_s, bytes_groups); - groups = (char *) g_malloc(bytes_groups + 1, 1); + if (bytes_groups > (sizeof(groups) - 1)) + { + LLOGLN(0, ("scard_process_list_readers: Unreasonable string length %u", + bytes_groups)); + return 1; + } in_uint8a(in_s, groups, bytes_groups); + groups[bytes_groups] = '\0'; in_uint32_le(in_s, cchReaders); LLOGLN(10, ("scard_process_list_readers: hContext 0x%8.8x cchReaders %d", hContext, cchReaders)); @@ -615,7 +630,6 @@ scard_process_list_readers(struct trans *con, struct stream *in_s) pcscListReaders->cchReaders = cchReaders; scard_send_list_readers(pcscListReaders, lcontext->context, lcontext->context_bytes, groups, cchReaders, 1); - g_free(groups); return 0; } diff --git a/xrdp/xrdp_bitmap.c b/xrdp/xrdp_bitmap.c index 1f5d010f..627f73c7 100644 --- a/xrdp/xrdp_bitmap.c +++ b/xrdp/xrdp_bitmap.c @@ -25,6 +25,8 @@ #include #endif +#include + #include "xrdp.h" #include "log.h" @@ -93,6 +95,30 @@ static const unsigned int g_crc_table[256] = (in_crc) = g_crc_table[((in_crc) ^ (in_pixel)) & 0xff] ^ ((in_crc) >> 8) #define CRC_END(in_crc) (in_crc) = ((in_crc) ^ 0xFFFFFFFF) +/*****************************************************************************/ +/* Allocate bitmap for specified dimensions, checking for int overflow */ +static char * +alloc_bitmap_data(int width, int height, int Bpp) +{ + char *result = NULL; + if (width > 0 && height > 0 && Bpp > 0) + { + int len = width; + /* g_malloc() currently takes an 'int' size */ + if (len < INT_MAX / height) + { + len *= height; + if (len < INT_MAX / Bpp) + { + len *= Bpp; + result = (char *)malloc(len); + } + } + } + + return result; +} + /*****************************************************************************/ struct xrdp_bitmap * xrdp_bitmap_create(int width, int height, int bpp, @@ -123,14 +149,28 @@ xrdp_bitmap_create(int width, int height, int bpp, if (self->type == WND_TYPE_BITMAP || self->type == WND_TYPE_IMAGE) { - self->data = (char *)g_malloc(width * height * Bpp, 0); + self->data = alloc_bitmap_data(width, height, Bpp); + if (self->data == NULL) + { + LLOGLN(0, ("xrdp_bitmap_create: size overflow %dx%dx%d", + width, height, Bpp)); + g_free(self); + return NULL; + } } #if defined(XRDP_PAINTER) if (self->type == WND_TYPE_SCREEN) /* noorders */ { LLOGLN(0, ("xrdp_bitmap_create: noorders")); - self->data = (char *) g_malloc(width * height * Bpp, 0); + self->data = alloc_bitmap_data(width, height, Bpp); + if (self->data == NULL) + { + LLOGLN(0, ("xrdp_bitmap_create: size overflow %dx%dx%d", + width, height, Bpp)); + g_free(self); + return NULL; + } } #endif diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index 184c60d9..a9ff3673 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -941,7 +941,7 @@ xrdp_wm_xor_pat(struct xrdp_wm *self, int x, int y, int cx, int cy) self->painter->brush.pattern[6] = 0xaa; self->painter->brush.pattern[7] = 0x55; self->painter->brush.x_origin = 0; - self->painter->brush.x_origin = 0; + self->painter->brush.y_origin = 0; self->painter->brush.style = 3; self->painter->bg_color = self->black; self->painter->fg_color = self->white; @@ -1560,6 +1560,12 @@ xrdp_wm_key(struct xrdp_wm *self, int device_flags, int scan_code) return 0; } + // workaround odd shift behavior + // see https://github.com/neutrinolabs/xrdp/issues/397 + if (scan_code == 42 && device_flags == (KBD_FLAG_UP | KBD_FLAG_EXT)) { + return 0; + } + if (device_flags & KBD_FLAG_UP) /* 0x8000 */ { self->keys[scan_code] = 0;