--- Begin Message ---
Hi all,
unfortunately this only made it through after xine-lib 1.1.2 release:
There has been a vulnerability report about libmms on [vendor-sec].
(CVE-2006-2200)
Please note that the original patch from the Debian maintainer is
partially incorrect (it should read memset(dest,0,2*len)), but the memset
isn't really necessary and could be nuked anyway. The use of memset in
the patch certainly doesn't do any harm, though, and it fixes the
potential overflow.
Luckily, xine uses libmms in a way that these vulnerabilities cannot be
exploited (buffers are large enough), and the xine module even seems to
rely on the side effects of the memset of the 'broken' library. Note
that the library sources are included (not an externally linked
library).
While analyzing the source I found a couple of potential heap overflows,
though, which I'm pretty sure that they can be exploited with some
effort. They are fixed in CVS. I also attached the according patch. But
I'm pretty sure that I overlooked some additional ones.
This source is a wormhole.
Sorry, Thibaut, but then you maybe coded the glue layer only :-]
Matthias
--
Matthias Hopf <[EMAIL PROTECTED]> __ __ __
Maxfeldstr. 5 / 90409 Nuernberg (_ | | (_ |__ [EMAIL PROTECTED]
Phone +49-911-74053-715 __) |_| __) |__ labs www.mshopf.de
diff -ur ../xine-lib-1.1.1.orig/src/input/mms.c ./src/input/mms.c
--- ../xine-lib-1.1.1.orig/src/input/mms.c 2005-04-21 21:02:43.000000000 +0200
+++ ./src/input/mms.c 2006-07-06 20:41:18.000000000 +0200
@@ -138,7 +138,7 @@
int num_stream_ids;
int stream_ids[ASF_MAX_NUM_STREAMS];
int stream_types[ASF_MAX_NUM_STREAMS];
- int asf_packet_len;
+ uint32_t asf_packet_len;
uint64_t file_len;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -371,13 +371,17 @@
goto error;
header->packet_len = LE_32(this->buf + 8) + 4;
+ if (header->packet_len > BUF_SIZE - 12) {
+ header->packet_len = 0;
+ goto error;
+ }
lprintf("mms command\n");
packet_type = MMS_PACKET_COMMAND;
} else {
header->packet_seq = LE_32(this->buf);
header->packet_id_type = this->buf[4];
header->flags = this->buf[5];
- header->packet_len = LE_16(this->buf + 6) - 8;
+ header->packet_len = (LE_16(this->buf + 6) - 8) & 0xffff;
if (header->packet_id_type == ASF_HEADER_PACKET_ID_TYPE) {
lprintf("asf header\n");
packet_type = MMS_PACKET_ASF_HEADER;
@@ -497,6 +501,11 @@
break;
case MMS_PACKET_ASF_HEADER:
case MMS_PACKET_ASF_PACKET:
+ if (header.packet_len + this->asf_header_len > ASF_HEADER_LEN) {
+ xprintf (this->stream->xine, XINE_VERBOSITY_LOG,
+ "libmms: asf packet too large\n");
+ return 0;
+ }
len = _x_io_tcp_read (this->stream, this->s,
this->asf_header + this->asf_header_len, header.packet_len);
if (len != header.packet_len) {
@@ -542,6 +551,12 @@
case GUID_ASF_FILE_PROPERTIES:
this->asf_packet_len = LE_32(this->asf_header + i + 92 - 24);
+ if (this->asf_packet_len > BUF_SIZE) {
+ this->asf_packet_len = 0;
+ xprintf (this->stream->xine, XINE_VERBOSITY_LOG,
+ "libmms: asf packet len too large\n");
+ break;
+ }
this->file_len = LE_64(this->asf_header + i + 40 - 24);
lprintf ("file object, file_length = %lld, packet length = %d",
this->file_len, this->asf_packet_len);
diff -ur ../xine-lib-1.1.1.orig/src/input/mmsh.c ./src/input/mmsh.c
--- ../xine-lib-1.1.1.orig/src/input/mmsh.c 2005-08-25 17:36:29.000000000 +0200
+++ ./src/input/mmsh.c 2006-07-06 20:05:58.000000000 +0200
@@ -182,7 +182,7 @@
int num_stream_ids;
int stream_ids[ASF_MAX_NUM_STREAMS];
int stream_types[ASF_MAX_NUM_STREAMS];
- int packet_length;
+ uint32_t packet_length;
int64_t file_length;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -491,6 +491,10 @@
case GUID_ASF_FILE_PROPERTIES:
this->packet_length = LE_32(this->asf_header + i + 92 - 24);
+ if (this->packet_length > CHUNK_SIZE) {
+ this->packet_length = 0;
+ break;
+ }
this->file_length = LE_64(this->asf_header + i + 40 - 24);
/*lprintf ("file object, file_length = %lld, packet length = %d",
this->file_length, this->packet_count);*/
--- End Message ---
--- Begin Message ---
Hi,
On Tue, Jul 11, 2006, Matthias Hopf wrote:
> Please note that the original patch from the Debian maintainer is
> partially incorrect (it should read memset(dest,0,2*len)), but the memset
> isn't really necessary and could be nuked anyway. The use of memset in
> the patch certainly doesn't do any harm, though, and it fixes the
> potential overflow.
[...]
> While analyzing the source I found a couple of potential heap overflows,
> though, which I'm pretty sure that they can be exploited with some
> effort. They are fixed in CVS. I also attached the according patch. But
> I'm pretty sure that I overlooked some additional ones.
Thanks for your review and for the additional fixes, I've adapted the
patch and added the memset() fixes you mention for the libmms source
package in Debian, I attach the incremental patch, "libmms_0.2-7.diff".
I also attach the cumulative patch of all successive patches I have
applied for CVE-2006-2200 to the libmms tree,
"libmms_0.2-7-cumulative.diff".
Bye,
--
Loïc Minier <[EMAIL PROTECTED]>
--- libmms-0.2/src/mms.c
+++ libmms-0.2/src/mms.c
@@ -144,7 +144,7 @@
int stream_types[ASF_MAX_NUM_STREAMS];
off_t start_packet_seq; /* for live streams != 0, need to keep it
around */
int need_discont; /* whether we need to set start_packet_seq */
- int asf_packet_len;
+ uint32_t asf_packet_len;
uint64_t file_len;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -477,7 +477,7 @@
}
static void string_utf16(iconv_t url_conv, char *dest, char *src, int len) {
- memset(dest, 0, len);
+ memset(dest, 0, 2 * len);
if (url_conv == (iconv_t)-1) {
int i;
@@ -503,7 +503,7 @@
static void string_utf16(int unused, char *dest, char *src, int len) {
int i;
- memset (dest, 0, len);
+ memset (dest, 0, 2 * len);
for (i = 0; i < len; i++) {
dest[i * 2] = src[i];
@@ -539,13 +539,17 @@
goto error;
header->packet_len = LE_32(this->buf + 8) + 4;
+ if (header->packet_len > BUF_SIZE - 12) {
+ header->packet_len = 0;
+ goto error;
+ }
lprintf("mms command\n");
packet_type = MMS_PACKET_COMMAND;
} else {
header->packet_seq = LE_32(this->buf);
header->packet_id_type = this->buf[4];
header->flags = this->buf[5];
- header->packet_len = LE_16(this->buf + 6) - 8;
+ header->packet_len = (LE_16(this->buf + 6) - 8) & 0xffff;
if (header->packet_id_type == ASF_HEADER_PACKET_ID_TYPE) {
lprintf("asf header\n");
packet_type = MMS_PACKET_ASF_HEADER;
@@ -674,6 +678,11 @@
break;
case MMS_PACKET_ASF_HEADER:
case MMS_PACKET_ASF_PACKET:
+ if (header.packet_len + this->asf_header_len > ASF_HEADER_LEN) {
+ lprintf( "***LOG:*** -- "
+ "libmms: asf packet too large\n");
+ return 0;
+ }
len = io_read(io, this->s,
this->asf_header + this->asf_header_len,
header.packet_len);
if (len != header.packet_len) {
@@ -720,6 +729,12 @@
case GUID_ASF_FILE_PROPERTIES:
this->asf_packet_len = LE_32(this->asf_header + i + 92 - 24);
+ if (this->asf_packet_len > BUF_SIZE) {
+ this->asf_packet_len = 0;
+ lprintf( "***LOG:*** -- "
+ "libmms: asf packet len too large\n");
+ break;
+ }
this->file_len = LE_64(this->asf_header + i + 40 - 24);
lprintf ("file object, packet length = %d (%d)\n",
this->asf_packet_len, LE_32(this->asf_header + i + 96 - 24));
--- libmms-0.2/src/mmsh.c
+++ libmms-0.2/src/mmsh.c
@@ -184,7 +184,7 @@
int num_stream_ids;
int stream_ids[ASF_MAX_NUM_STREAMS];
int stream_types[ASF_MAX_NUM_STREAMS];
- int packet_length;
+ uint32_t packet_length;
int64_t file_length;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -604,6 +604,10 @@
case GUID_ASF_FILE_PROPERTIES:
this->packet_length = LE_32(this->asf_header + i + 92 - 24);
+ if (this->packet_length > CHUNK_SIZE) {
+ this->packet_length = 0;
+ break;
+ }
this->file_length = LE_64(this->asf_header + i + 40 - 24);
lprintf ("file object, packet length = %d (%d)\n",
this->packet_length, LE_32(this->asf_header + i + 96 - 24));
--- libmms-0.2/debian/changelog
+++ libmms-0.2/debian/changelog
@@ -1,3 +1,15 @@
+libmms (0.2-7) unstable; urgency=high
+
+ * SECURITY: CVE-2006-2200: buffer overflows in mms / mmsh parsers:
+ additional fixes thanks to Matthias Hopf:
+ - even more checks on "packet_length" / "packet_len" in src/mms.c and
+ src/mmsh.c
+ - fix memset() calls in the two string_utf16() implementations in
+ src/mms.c to clear all bytes in dest, "len" is the UTF-16 length of the
+ string in wide chars, so the memset should use "2 * len".
+
+ -- Loic Minier <[EMAIL PROTECTED]> Tue, 11 Jul 2006 13:11:11 +0200
+
libmms (0.2-6) unstable; urgency=low
* SECURITY: CVE-2006-2200: buffer overflows in mms / mmsh parsers: fix an
--- libmms-0.2/debian/changelog
+++ libmms-0.2/debian/changelog
@@ -1,3 +1,30 @@
+libmms (0.2-7) unstable; urgency=high
+
+ * SECURITY: CVE-2006-2200: buffer overflows in mms / mmsh parsers:
+ additional fixes thanks to Matthias Hopf:
+ - even more checks on "packet_length" / "packet_len" in src/mms.c and
+ src/mmsh.c
+ - fix memset() calls in the two string_utf16() implementations in
+ src/mms.c to clear all bytes in dest, "len" is the UTF-16 length of the
+ string in wide chars, so the memset should use "2 * len".
+
+ -- Loic Minier <[EMAIL PROTECTED]> Tue, 11 Jul 2006 13:11:11 +0200
+
+libmms (0.2-6) unstable; urgency=low
+
+ * SECURITY: CVE-2006-2200: buffer overflows in mms / mmsh parsers: fix an
+ error that crept in the previous fix and use start < end instead of start
+ > end in src/mms.c and src/mmsh.c, thanks Martin Pitt.
+
+ -- Loic Minier <[EMAIL PROTECTED]> Wed, 5 Jul 2006 18:13:36 +0200
+
+libmms (0.2-5) unstable; urgency=high
+
+ * SECURITY: CVE-2006-2200: buffer overflows in mms / mmsh parsers: apply
+ memset() range fixes adapted from #374577 by Wesley J. Landaker.
+
+ -- Loic Minier <[EMAIL PROTECTED]> Thu, 22 Jun 2006 20:53:44 +0200
+
libmms (0.2-4) unstable; urgency=low
* Apply patch from Wesley J. Landaker for the headers to be usable from C++.
--- libmms-0.2.orig/src/mms.c
+++ libmms-0.2/src/mms.c
@@ -144,7 +144,7 @@
int stream_types[ASF_MAX_NUM_STREAMS];
off_t start_packet_seq; /* for live streams != 0, need to keep it
around */
int need_discont; /* whether we need to set start_packet_seq */
- int asf_packet_len;
+ uint32_t asf_packet_len;
uint64_t file_len;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -477,7 +477,7 @@
}
static void string_utf16(iconv_t url_conv, char *dest, char *src, int len) {
- memset(dest, 0, 1000);
+ memset(dest, 0, 2 * len);
if (url_conv == (iconv_t)-1) {
int i;
@@ -503,7 +503,7 @@
static void string_utf16(int unused, char *dest, char *src, int len) {
int i;
- memset (dest, 0, 1000);
+ memset (dest, 0, 2 * len);
for (i = 0; i < len; i++) {
dest[i * 2] = src[i];
@@ -539,13 +539,17 @@
goto error;
header->packet_len = LE_32(this->buf + 8) + 4;
+ if (header->packet_len > BUF_SIZE - 12) {
+ header->packet_len = 0;
+ goto error;
+ }
lprintf("mms command\n");
packet_type = MMS_PACKET_COMMAND;
} else {
header->packet_seq = LE_32(this->buf);
header->packet_id_type = this->buf[4];
header->flags = this->buf[5];
- header->packet_len = LE_16(this->buf + 6) - 8;
+ header->packet_len = (LE_16(this->buf + 6) - 8) & 0xffff;
if (header->packet_id_type == ASF_HEADER_PACKET_ID_TYPE) {
lprintf("asf header\n");
packet_type = MMS_PACKET_ASF_HEADER;
@@ -674,6 +678,11 @@
break;
case MMS_PACKET_ASF_HEADER:
case MMS_PACKET_ASF_PACKET:
+ if (header.packet_len + this->asf_header_len > ASF_HEADER_LEN) {
+ lprintf( "***LOG:*** -- "
+ "libmms: asf packet too large\n");
+ return 0;
+ }
len = io_read(io, this->s,
this->asf_header + this->asf_header_len,
header.packet_len);
if (len != header.packet_len) {
@@ -720,6 +729,12 @@
case GUID_ASF_FILE_PROPERTIES:
this->asf_packet_len = LE_32(this->asf_header + i + 92 - 24);
+ if (this->asf_packet_len > BUF_SIZE) {
+ this->asf_packet_len = 0;
+ lprintf( "***LOG:*** -- "
+ "libmms: asf packet len too large\n");
+ break;
+ }
this->file_len = LE_64(this->asf_header + i + 40 - 24);
lprintf ("file object, packet length = %d (%d)\n",
this->asf_packet_len, LE_32(this->asf_header + i + 96 - 24));
@@ -1420,8 +1435,20 @@
/* explicit padding with 0 */
lprintf("padding: %d bytes\n", this->asf_packet_len -
header.packet_len);
- memset(this->buf + header.packet_len, 0, this->asf_packet_len -
header.packet_len);
- this->buf_size = this->asf_packet_len;
+ {
+ char *base = (char *)(this->buf);
+ char *start = base + header.packet_len;
+ char *end = start + this->asf_packet_len - header.packet_len;
+ if ((start > base) && (start < (base+BUF_SIZE-1)) &&
+ (start < end) && (end < (base+BUF_SIZE-1))) {
+ memset(this->buf + header.packet_len, 0, this->asf_packet_len -
header.packet_len);
+ }
+ if (this->asf_packet_len > BUF_SIZE) {
+ this->buf_size = BUF_SIZE;
+ } else {
+ this->buf_size = this->asf_packet_len;
+ }
+ }
}
break;
}
--- libmms-0.2.orig/src/mmsh.c
+++ libmms-0.2/src/mmsh.c
@@ -184,7 +184,7 @@
int num_stream_ids;
int stream_ids[ASF_MAX_NUM_STREAMS];
int stream_types[ASF_MAX_NUM_STREAMS];
- int packet_length;
+ uint32_t packet_length;
int64_t file_length;
char guid[37];
uint32_t bitrates[ASF_MAX_NUM_STREAMS];
@@ -604,6 +604,10 @@
case GUID_ASF_FILE_PROPERTIES:
this->packet_length = LE_32(this->asf_header + i + 92 - 24);
+ if (this->packet_length > CHUNK_SIZE) {
+ this->packet_length = 0;
+ break;
+ }
this->file_length = LE_64(this->asf_header + i + 40 - 24);
lprintf ("file object, packet length = %d (%d)\n",
this->packet_length, LE_32(this->asf_header + i + 96 - 24));
@@ -1054,9 +1058,22 @@
this->chunk_length, this->packet_length);
return 0;
}
- memset(this->buf + this->chunk_length, 0,
- this->packet_length - this->chunk_length);
- this->buf_size = this->packet_length;
+
+ {
+ char *base = (char *)(this->buf);
+ char *start = base + this->chunk_length;
+ char *end = start + this->packet_length - this->chunk_length;
+ if ((start > base) && (start < (base+CHUNK_SIZE-1)) &&
+ (start < end) && (end < (base+CHUNK_SIZE-1))) {
+ memset(start, 0,
+ this->packet_length - this->chunk_length);
+ }
+ if (this->packet_length > CHUNK_SIZE) {
+ this->buf_size = CHUNK_SIZE;
+ } else {
+ this->buf_size = this->packet_length;
+ }
+ }
return 1;
} else {
lprintf ("mmsh: read error, %d != %d\n", len, this->chunk_length);
--- End Message ---
