From 9af5c814fa027fafcc9dfd7abdc4ffba223d1a33 Mon Sep 17 00:00:00 2001 From: Jay Sorg Date: Thu, 5 Sep 2013 12:56:12 -0700 Subject: [PATCH] VUL: fix some possible buffer overruns --- common/parse.h | 3 ++ common/trans.c | 5 +++ libxrdp/xrdp_iso.c | 16 +++++++++ libxrdp/xrdp_mcs.c | 83 +++++++++++++++++++++++++++++++++++++++++++++- libxrdp/xrdp_sec.c | 44 +++++++++++++++++++++--- 5 files changed, 146 insertions(+), 5 deletions(-) diff --git a/common/parse.h b/common/parse.h index 9bd6850c..41cfa698 100644 --- a/common/parse.h +++ b/common/parse.h @@ -59,6 +59,9 @@ struct stream /******************************************************************************/ #define s_check_rem(s, n) ((s)->p + (n) <= (s)->end) +/******************************************************************************/ +#define s_check_rem_out(s, n) ((s)->p + (n) <= (s)->data + (s)->size) + /******************************************************************************/ #define s_check_end(s) ((s)->p == (s)->end) diff --git a/common/trans.c b/common/trans.c index 377b90ba..4aa8917e 100644 --- a/common/trans.c +++ b/common/trans.c @@ -202,6 +202,11 @@ trans_force_read_s(struct trans* self, struct stream* in_s, int size) } while (size > 0) { + /* make sure stream has room */ + if ((in_s->end + size) > (in_s->data + in_s->size)) + { + return 1; + } rcvd = g_tcp_recv(self->sck, in_s->end, size, 0); if (rcvd == -1) { diff --git a/libxrdp/xrdp_iso.c b/libxrdp/xrdp_iso.c index 7fee92e6..983bfb9a 100644 --- a/libxrdp/xrdp_iso.c +++ b/libxrdp/xrdp_iso.c @@ -68,18 +68,34 @@ xrdp_iso_recv_msg(struct xrdp_iso* self, struct stream* s, int* code) } in_uint8s(s, 1); in_uint16_be(s, len); + if (len < 4) + { + return 1; + } if (xrdp_tcp_recv(self->tcp_layer, s, len - 4) != 0) { return 1; } + if (!s_check_rem(s, 2)) + { + return 1; + } in_uint8s(s, 1); in_uint8(s, *code); if (*code == ISO_PDU_DT) { + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8s(s, 1); } else { + if (!s_check_rem(s, 5)) + { + return 1; + } in_uint8s(s, 5); } return 0; diff --git a/libxrdp/xrdp_mcs.c b/libxrdp/xrdp_mcs.c index af65faf2..06a38418 100644 --- a/libxrdp/xrdp_mcs.c +++ b/libxrdp/xrdp_mcs.c @@ -120,6 +120,10 @@ xrdp_mcs_recv(struct xrdp_mcs* self, struct stream* s, int* chan) DEBUG((" out xrdp_mcs_recv xrdp_iso_recv returned non zero")); return 1; } + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8(s, opcode); appid = opcode >> 2; if (appid == MCS_DPUM) @@ -130,6 +134,10 @@ xrdp_mcs_recv(struct xrdp_mcs* self, struct stream* s, int* chan) /* this is channels getting added from the client */ if (appid == MCS_CJRQ) { + if (!s_check_rem(s, 4)) + { + return 1; + } in_uint16_be(s, userid); in_uint16_be(s, chanid); DEBUG((" adding channel %4.4x", chanid)); @@ -143,12 +151,20 @@ xrdp_mcs_recv(struct xrdp_mcs* self, struct stream* s, int* chan) DEBUG((" out xrdp_mcs_recv err got 0x%x need MCS_SDRQ", appid)); return 1; } + if (!s_check_rem(s, 6)) + { + return 1; + } in_uint8s(s, 2); in_uint16_be(s, *chan); in_uint8s(s, 1); in_uint8(s, len); if (len & 0x80) { + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8s(s, 1); } DEBUG((" out xrdp_mcs_recv")); @@ -167,16 +183,28 @@ xrdp_mcs_ber_parse_header(struct xrdp_mcs* self, struct stream* s, if (tag_val > 0xff) { + if (!s_check_rem(s, 2)) + { + return 1; + } in_uint16_be(s, tag); } else { + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8(s, tag); } if (tag != tag_val) { return 1; } + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8(s, l); if (l & 0x80) { @@ -184,6 +212,10 @@ xrdp_mcs_ber_parse_header(struct xrdp_mcs* self, struct stream* s, *len = 0; while (l > 0) { + if (!s_check_rem(s, 1)) + { + return 1; + } in_uint8(s, i); *len = (*len << 8) | i; l--; @@ -214,6 +246,10 @@ xrdp_mcs_parse_domain_params(struct xrdp_mcs* self, struct stream* s) { return 1; } + if (!s_check_rem(s, len)) + { + return 1; + } in_uint8s(s, len); if (s_check(s)) { @@ -234,7 +270,7 @@ xrdp_mcs_recv_connect_initial(struct xrdp_mcs* self) struct stream* s; make_stream(s); - init_stream(s, 8192); + init_stream(s, 16 * 1024); if (xrdp_iso_recv(self->iso_layer, s) != 0) { free_stream(s); @@ -283,6 +319,11 @@ xrdp_mcs_recv_connect_initial(struct xrdp_mcs* self) free_stream(s); return 1; } + if ((len <= 0) || (len > 16 * 1024)) + { + free_stream(s); + return 1; + } /* make a copy of client mcs data */ init_stream(self->client_mcs_data, len); out_uint8a(self->client_mcs_data, s->p, len); @@ -315,16 +356,31 @@ xrdp_mcs_recv_edrq(struct xrdp_mcs* self) free_stream(s); return 1; } + if (!s_check_rem(s, 1)) + { + free_stream(s); + return 1; + } in_uint8(s, opcode); if ((opcode >> 2) != MCS_EDRQ) { free_stream(s); return 1; } + if (!s_check_rem(s, 4)) + { + free_stream(s); + return 1; + } in_uint8s(s, 2); in_uint8s(s, 2); if (opcode & 2) { + if (!s_check_rem(s, 2)) + { + free_stream(s); + return 1; + } in_uint16_be(s, self->userid); } if (!(s_check_end(s))) @@ -351,6 +407,11 @@ xrdp_mcs_recv_aurq(struct xrdp_mcs* self) free_stream(s); return 1; } + if (!s_check_rem(s, 1)) + { + free_stream(s); + return 1; + } in_uint8(s, opcode); if ((opcode >> 2) != MCS_AURQ) { @@ -359,6 +420,11 @@ xrdp_mcs_recv_aurq(struct xrdp_mcs* self) } if (opcode & 2) { + if (!s_check_rem(s, 2)) + { + free_stream(s); + return 1; + } in_uint16_be(s, self->userid); } if (!(s_check_end(s))) @@ -416,15 +482,30 @@ xrdp_mcs_recv_cjrq(struct xrdp_mcs* self) free_stream(s); return 1; } + if (!s_check_rem(s, 1)) + { + free_stream(s); + return 1; + } in_uint8(s, opcode); if ((opcode >> 2) != MCS_CJRQ) { free_stream(s); return 1; } + if (!s_check_rem(s, 4)) + { + free_stream(s); + return 1; + } in_uint8s(s, 4); if (opcode & 2) { + if (!s_check_rem(s, 2)) + { + free_stream(s); + return 1; + } in_uint8s(s, 2); } if (!(s_check_end(s))) diff --git a/libxrdp/xrdp_sec.c b/libxrdp/xrdp_sec.c index 983be09f..4f78b0e9 100644 --- a/libxrdp/xrdp_sec.c +++ b/libxrdp/xrdp_sec.c @@ -724,15 +724,27 @@ xrdp_sec_process_mcs_data_channels(struct xrdp_sec* self, struct stream* s) { return 0; } + if (!s_check_rem(s, 4)) + { + return 1; + } in_uint32_le(s, num_channels); + if (num_channels > 256) + { + return 1; + } for (index = 0; index < num_channels; index++) { channel_item = (struct mcs_channel_item*) g_malloc(sizeof(struct mcs_channel_item), 1); + if (!s_check_rem(s, 12)) + { + return 1; + } in_uint8a(s, channel_item->name, 8); in_uint32_be(s, channel_item->flags); channel_item->chanid = MCS_GLOBAL_CHANNEL + (index + 1); - list_add_item(self->mcs_layer->channel_list, (long)channel_item); + list_add_item(self->mcs_layer->channel_list, (tintptr)channel_item); DEBUG(("got channel flags %8.8x name %s", channel_item->flags, channel_item->name)); } @@ -750,10 +762,14 @@ xrdp_sec_process_mcs_data(struct xrdp_sec* self) int tag = 0; int size = 0; - s = &self->client_mcs_data; + s = &(self->client_mcs_data); /* set p to beginning */ s->p = s->data; /* skip header */ + if (!s_check_rem(s, 23)) + { + return 1; + } in_uint8s(s, 23); while (s_check_rem(s, 4)) { @@ -878,7 +894,7 @@ xrdp_sec_out_mcs_data(struct xrdp_sec* self) /*****************************************************************************/ /* process the mcs client data we received from the mcs layer */ -static void APP_CC +static int APP_CC xrdp_sec_in_mcs_data(struct xrdp_sec* self) { struct stream* s = (struct stream *)NULL; @@ -890,12 +906,20 @@ xrdp_sec_in_mcs_data(struct xrdp_sec* self) s = &(self->client_mcs_data); /* get hostname, its unicode */ s->p = s->data; + if (!s_check_rem(s, 47)) + { + return 1; + } in_uint8s(s, 47); g_memset(client_info->hostname, 0, 32); c = 1; index = 0; while (index < 16 && c != 0) { + if (!s_check_rem(s, 2)) + { + return 1; + } in_uint8(s, c); in_uint8s(s, 1); client_info->hostname[index] = c; @@ -903,13 +927,22 @@ xrdp_sec_in_mcs_data(struct xrdp_sec* self) } /* get build */ s->p = s->data; + if (!s_check_rem(s, 43 + 4)) + { + return 1; + } in_uint8s(s, 43); in_uint32_le(s, client_info->build); /* get keylayout */ s->p = s->data; + if (!s_check_rem(s, 39 + 4)) + { + return 1; + } in_uint8s(s, 39); in_uint32_le(s, client_info->keylayout); s->p = s->data; + return 0; } /*****************************************************************************/ @@ -976,7 +1009,10 @@ xrdp_sec_incoming(struct xrdp_sec* self) (int)(self->server_mcs_data.end - self->server_mcs_data.data)); #endif DEBUG((" out xrdp_sec_incoming")); - xrdp_sec_in_mcs_data(self); + if (xrdp_sec_in_mcs_data(self) != 0) + { + return 1; + } return 0; }