From 6310414eccaadf292b3b32a4423ebf5c1e3e7255 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Sun, 4 Jan 2009 17:21:46 +0000 Subject: Fix for CVE-2008-5234. Multiple heap-based buffer overflows in xine-lib 1.1.12, and other versions before 1.1.15, allow remote attackers to execute arbitrary code via vectors related to (1) a crafted metadata atom size processed by the parse_moov_atom function in demux_qt.c and (2) frame reading in the id3v23_interp_frame function in id3.c. NOTE: as of 20081122, it is possible that vector 1 has not been fixed in 1.1.15. case ( FOURCC_TAG('C', 'O', 'M', 'M') ): _x_meta_info_set_generic(stream, XINE_META_INFO_COMMENT, buf + 1 + 3, id3_encoding[enc]); --- src/demuxers/demux_qt.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_qt.c b/src/demuxers/demux_qt.c index 1fa9b4327..3d4161fbf 100644 --- a/src/demuxers/demux_qt.c +++ b/src/demuxers/demux_qt.c @@ -738,6 +738,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { if (current_atom == ART_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->artist = xine_xmalloc(string_size); if (info->artist) { strncpy(info->artist, &meta_atom[i + 20], string_size - 1); @@ -745,6 +747,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == NAM_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->name = xine_xmalloc(string_size); if (info->name) { strncpy(info->name, &meta_atom[i + 20], string_size - 1); @@ -752,6 +756,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == ALB_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->album = xine_xmalloc(string_size); if (info->album) { strncpy(info->album, &meta_atom[i + 20], string_size - 1); @@ -759,6 +765,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == GEN_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->genre = xine_xmalloc(string_size); if (info->genre) { strncpy(info->genre, &meta_atom[i + 20], string_size - 1); @@ -766,6 +774,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == TOO_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->comment = xine_xmalloc(string_size); if (info->comment) { strncpy(info->comment, &meta_atom[i + 20], string_size - 1); @@ -773,6 +783,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == WRT_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->composer = xine_xmalloc(string_size); if (info->composer) { strncpy(info->composer, &meta_atom[i + 20], string_size - 1); @@ -780,6 +792,8 @@ static void parse_meta_atom(qt_info *info, unsigned char *meta_atom) { } } else if (current_atom == DAY_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; + if (string_size <= 0) + continue; info->year = xine_xmalloc(string_size); if (info->year) { strncpy(info->year, &meta_atom[i + 20], string_size - 1); @@ -947,6 +961,10 @@ static qt_error parse_trak_atom (qt_trak *trak, /* allocate space for each of the properties unions */ trak->stsd_atoms_count = _X_BE_32(&trak_atom[i + 8]); + if (trak->stsd_atoms_count <= 0) { + last_error = QT_HEADER_TROUBLE; + goto free_trak; + } trak->stsd_atoms = calloc(trak->stsd_atoms_count, sizeof(properties_t)); if (!trak->stsd_atoms) { last_error = QT_NO_MEMORY; @@ -958,6 +976,10 @@ static qt_error parse_trak_atom (qt_trak *trak, for (k = 0; k < trak->stsd_atoms_count; k++) { current_stsd_atom_size = _X_BE_32(&trak_atom[atom_pos - 4]); + if (current_stsd_atom_size < 4) { + last_error = QT_HEADER_TROUBLE; + goto free_trak; + } if (trak->type == MEDIA_VIDEO) { -- cgit v1.2.3 From b108d0826de0fc395a0d3eb2e5612a20df6cf334 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Sun, 4 Jan 2009 17:21:46 +0000 Subject: Fix for CVE-2008-5236. Multiple heap-based buffer overflows in xine-lib 1.1.12, and other 1.1.15 and earlier versions, allow remote attackers to execute arbitrary code via vectors related to (1) a crafted EBML element length processed by the parse_block_group function in demux_matroska.c; (2) a certain combination of sps, w, and h values processed by the real_parse_audio_specific_data and demux_real_send_chunk functions in demux_real.c; and (3) an unspecified combination of three values processed by the open_ra_file function in demux_realaudio.c. NOTE: vector 2 reportedly exists because of an incomplete fix in 1.1.15. --- src/demuxers/demux_matroska.c | 7 ++++++- src/demuxers/demux_real.c | 9 +++++++-- src/demuxers/demux_realaudio.c | 18 +++++++++++++----- 3 files changed, 26 insertions(+), 8 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_matroska.c b/src/demuxers/demux_matroska.c index a8af6c44f..6bd700f9a 100644 --- a/src/demuxers/demux_matroska.c +++ b/src/demuxers/demux_matroska.c @@ -1179,7 +1179,12 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { break; case MATROSKA_ID_TR_CODECPRIVATE: { - char *codec_private = malloc (elem.len); + char *codec_private; + if (elem.len >= 0x80000000) + return 0; + codec_private = malloc (elem.len); + if (! codec_private) + return 0; lprintf("CodecPrivate\n"); if (!ebml_read_binary(ebml, &elem, codec_private)) { free(codec_private); diff --git a/src/demuxers/demux_real.c b/src/demuxers/demux_real.c index 32b516537..965470125 100644 --- a/src/demuxers/demux_real.c +++ b/src/demuxers/demux_real.c @@ -359,9 +359,14 @@ static void real_parse_audio_specific_data (demux_real_t *this, * stream->frame_size = stream->w / stream->sps * stream->h * stream->sps; * but it looks pointless? the compiler will probably optimise it away, I suppose? */ - stream->frame_size = stream->w * stream->h; + if (stream->w < 32768 && stream->h < 32768) { + stream->frame_size = stream->w * stream->h; + stream->frame_buffer = calloc(stream->frame_size, 1); + } else { + stream->frame_size = 0; + stream->frame_buffer = NULL; + } - stream->frame_buffer = calloc(stream->frame_size, 1); stream->frame_num_bytes = 0; stream->sub_packet_cnt = 0; diff --git a/src/demuxers/demux_realaudio.c b/src/demuxers/demux_realaudio.c index 1c39c6208..44449667c 100644 --- a/src/demuxers/demux_realaudio.c +++ b/src/demuxers/demux_realaudio.c @@ -202,11 +202,19 @@ static int open_ra_file(demux_ra_t *this) { this->h = _X_BE_16 (this->header+40); this->cfs = _X_BE_32 (this->header+24); - this->frame_len = this->w * this->h; - this->frame_size = this->frame_len * sps; - - this->frame_buffer = calloc(this->frame_size, 1); - _x_assert(this->frame_buffer != NULL); + if (this->w < 0x8000 && this->h < 0x8000) { + uint64_t fs; + this->frame_len = this->w * this->h; + fs = (uint64_t) this->frame_len * sps; + if (fs < 0x80000000) { + this->frame_size = fs; + this->frame_buffer = calloc(this->frame_size, 1); + } + } + if (! this->frame_buffer) { + xprintf(this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_realaudio: malloc failed\n"); + return 0; + } if (this->audio_type == BUF_AUDIO_28_8 || this->audio_type == BUF_AUDIO_SIPRO) this->block_align = this->cfs; -- cgit v1.2.3 From 49d4eec32b8c05eadfaf9c42b3dcd7407815fd9a Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Sun, 4 Jan 2009 17:21:46 +0000 Subject: Fix for CVE-2008-5237 Multiple integer overflows in xine-lib 1.1.12, and other 1.1.15 and earlier versions, allow remote attackers to cause a denial of service (crash) or possibly execute arbitrary code via (1) crafted width and height values that are not validated by the mymng_process_header function in demux_mng.c before use in an allocation calculation or (2) crafted current_atom_size and string_size values processed by the parse_reference_atom function in demux_qt.c. --- src/demuxers/demux_mng.c | 3 +++ src/demuxers/demux_qt.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mng.c b/src/demuxers/demux_mng.c index 0fcdb24ff..b57a349c5 100644 --- a/src/demuxers/demux_mng.c +++ b/src/demuxers/demux_mng.c @@ -112,6 +112,9 @@ static mng_bool mymng_read_stream(mng_handle mngh, mng_ptr buffer, mng_uint32 si static mng_bool mymng_process_header(mng_handle mngh, mng_uint32 width, mng_uint32 height){ demux_mng_t *this = (demux_mng_t*)mng_get_userdata(mngh); + if (width > 0x8000 || height > 0x8000) + return MNG_FALSE; + this->bih.biWidth = (width + 7) & ~7; this->bih.biHeight = height; this->left_edge = (this->bih.biWidth - width) / 2; diff --git a/src/demuxers/demux_qt.c b/src/demuxers/demux_qt.c index 3d4161fbf..c569ef9a4 100644 --- a/src/demuxers/demux_qt.c +++ b/src/demuxers/demux_qt.c @@ -1597,13 +1597,16 @@ static qt_error parse_reference_atom (reference_t *ref, qt_atom current_atom; unsigned int current_atom_size; + if (ref_atom_size >= 0x80000000) + return QT_NOT_A_VALID_FILE; + /* initialize reference atom */ ref->url = NULL; ref->data_rate = 0; ref->qtim_version = 0; /* traverse through the atom looking for the key atoms */ - for (i = ATOM_PREAMBLE_SIZE; i < ref_atom_size - 4; i++) { + for (i = ATOM_PREAMBLE_SIZE; i + 4 < ref_atom_size; i++) { current_atom_size = _X_BE_32(&ref_atom[i - 4]); current_atom = _X_BE_32(&ref_atom[i]); @@ -1612,7 +1615,7 @@ static qt_error parse_reference_atom (reference_t *ref, size_t string_size = _X_BE_32(&ref_atom[i + 12]); size_t url_offset = 0; - if (string_size >= current_atom_size || i + string_size >= ref_atom_size) + if (string_size >= current_atom_size || string_size >= ref_atom_size - i) return QT_NOT_A_VALID_FILE; /* if the URL starts with "http://", copy it */ @@ -1620,6 +1623,8 @@ static qt_error parse_reference_atom (reference_t *ref, memcmp(&ref_atom[i + 16], "rtsp://", 7) && base_mrl ) url_offset = strlen(base_mrl); + if (url_offset >= 0x80000000) + return QT_NOT_A_VALID_FILE; /* otherwise, append relative URL to base MRL */ string_size += url_offset; -- cgit v1.2.3 From ba5f2ab8d7209f3971ecf22ea3bc5ee43a692b5c Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Sun, 4 Jan 2009 17:21:46 +0000 Subject: Fix for CVE-2008-5240 xine-lib 1.1.12, and other 1.1.15 and earlier versions, relies on an untrusted input value to determine the memory allocation and does not check the result for (1) the MATROSKA_ID_TR_CODECPRIVATE track entry element processed by demux_matroska.c; and (2) PROP_TAG, (3) MDPR_TAG, and (4) CONT_TAG chunks processed by the real_parse_headers function in demux_real.c; which allows remote attackers to cause a denial of service (NULL pointer dereference and crash) or possibly execute arbitrary code via a crafted value. --- src/demuxers/demux_real.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_real.c b/src/demuxers/demux_real.c index 965470125..efb39b75f 100644 --- a/src/demuxers/demux_real.c +++ b/src/demuxers/demux_real.c @@ -435,9 +435,14 @@ static void real_parse_headers (demux_real_t *this) { case MDPR_TAG: case CONT_TAG: { + if (chunk_size < PREAMBLE_SIZE+1) { + this->status = DEMUX_FINISHED; + return; + } chunk_size -= PREAMBLE_SIZE; uint8_t *const chunk_buffer = malloc(chunk_size); - if (this->input->read(this->input, chunk_buffer, chunk_size) != + if (! chunk_buffer || + this->input->read(this->input, chunk_buffer, chunk_size) != chunk_size) { free (chunk_buffer); this->status = DEMUX_FINISHED; -- cgit v1.2.3 From de3b12bc7488ca43834f2840619744a9ec2c5b16 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Sun, 4 Jan 2009 17:21:46 +0000 Subject: Fix for CVE-2008-5243. The real_parse_headers function in demux_real.c in xine-lib 1.1.12, and other 1.1.15 and earlier versions, relies on an untrusted input length value to "reindex into an allocated buffer," which allows remote attackers to cause a denial of service (crash) via a crafted value, probably an array index error. --- src/demuxers/demux_real.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_real.c b/src/demuxers/demux_real.c index efb39b75f..2b0153450 100644 --- a/src/demuxers/demux_real.c +++ b/src/demuxers/demux_real.c @@ -497,7 +497,8 @@ static void real_parse_headers (demux_real_t *this) { this->audio_streams[this->num_audio_streams].index = NULL; this->audio_streams[this->num_audio_streams].mdpr = mdpr; this->num_audio_streams++; - } else if(_X_BE_32(mdpr->type_specific_data) == RA_TAG) { + } else if(_X_BE_32(mdpr->type_specific_data) == RA_TAG && + mdpr->type_specific_len >= 6) { if(this->num_audio_streams == MAX_AUDIO_STREAMS) { xprintf(this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_real: maximum number of audio stream exceeded\n"); @@ -508,26 +509,30 @@ static void real_parse_headers (demux_real_t *this) { lprintf("audio version %d detected\n", version); - char *fourcc_ptr = NULL; + char *fourcc_ptr = "\0\0\0"; switch(version) { case 3: /* Version 3 header stores fourcc after meta info - cheat by reading backwards from the * end of the header instead of having to parse it all */ - fourcc_ptr = mdpr->type_specific_data + mdpr->type_specific_len - 5; + if (mdpr->type_specific_len >= 5) + fourcc_ptr = mdpr->type_specific_data + mdpr->type_specific_len - 5; break; case 4: { - const uint8_t len = *(mdpr->type_specific_data + 56); - fourcc_ptr = mdpr->type_specific_data + 58 + len; + if (mdpr->type_specific_len >= 57) { + const uint8_t len = *(mdpr->type_specific_data + 56); + if (mdpr->type_specific_len >= 62 + len) + fourcc_ptr = mdpr->type_specific_data + 58 + len; + } } break; case 5: - fourcc_ptr = mdpr->type_specific_data + 66; + if (mdpr->type_specific_len >= 70) + fourcc_ptr = mdpr->type_specific_data + 66; break; default: lprintf("unsupported audio header version %d\n", version); goto unknown; } - lprintf("fourcc = %.4s\n", fourcc_ptr); const uint32_t fourcc = _X_ME_32(fourcc_ptr); -- cgit v1.2.3 From 4f32e89e05204737b233617fd065a1da519bfbf8 Mon Sep 17 00:00:00 2001 From: Thomas Viehmann Date: Sun, 4 Jan 2009 21:11:44 +0100 Subject: fail to set up codec when fifo is not set up When a track's fifo is not set up (typically because the track type is invalid), do not call init_codec, as all implementations dereference track->fifo, segfaulting if it is NULL. --- src/demuxers/demux_matroska.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_matroska.c b/src/demuxers/demux_matroska.c index 6bd700f9a..0a96295ec 100644 --- a/src/demuxers/demux_matroska.c +++ b/src/demuxers/demux_matroska.c @@ -1484,9 +1484,14 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { break; } - if (init_codec) + if (init_codec) { + if (! track->fifo) { + xprintf(this->stream->xine, XINE_VERBOSITY_LOG, + "demux_matroska: Error: fifo not set up for track of type type %" PRIu32 "\n", track->track_type); + return 0; + } init_codec(this, track); - + } } } -- cgit v1.2.3 From 0d4380861db644bcd758d596c638886b1578f601 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 19:22:16 +0100 Subject: handle read errors when forwarding in multiple demuxers Add checks for negative return values in aac,ac3,dts,mpc, nsf,ogg,shn,slave,ts,tta,vox demuxers. Some input plugins (e.g. file) return negative error codes from read, this should be treated as no (more) data available. This is particularly the negative size is then assigned to buf->size, potentially causing overflows elsewhere. The patch also removes the duplication of the (previously) == 0 handler in demux_ac3. --- src/demuxers/demux_aac.c | 2 +- src/demuxers/demux_ac3.c | 8 +------- src/demuxers/demux_dts.c | 2 +- src/demuxers/demux_mpc.c | 2 +- src/demuxers/demux_nsf.c | 2 +- src/demuxers/demux_ogg.c | 2 +- src/demuxers/demux_shn.c | 2 +- src/demuxers/demux_slave.c | 5 +++-- src/demuxers/demux_ts.c | 2 +- src/demuxers/demux_tta.c | 4 ++++ src/demuxers/demux_vox.c | 2 +- 11 files changed, 16 insertions(+), 17 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_aac.c b/src/demuxers/demux_aac.c index b8e6ec5c4..68a684ffa 100644 --- a/src/demuxers/demux_aac.c +++ b/src/demuxers/demux_aac.c @@ -173,7 +173,7 @@ static int demux_aac_send_chunk(demux_plugin_t *this_gen) { buf->extra_info->input_time = (8*current_pos) / (bitrate/1000); bytes_read = this->input->read(this->input, buf->content, buf->max_size); - if (bytes_read == 0) { + if (bytes_read <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; diff --git a/src/demuxers/demux_ac3.c b/src/demuxers/demux_ac3.c index 1acb12fcd..711bb0b45 100644 --- a/src/demuxers/demux_ac3.c +++ b/src/demuxers/demux_ac3.c @@ -311,13 +311,7 @@ static int demux_ac3_send_chunk (demux_plugin_t *this_gen) { this->frame_size); } - if (buf->size == 0) { - buf->free_buffer(buf); - this->status = DEMUX_FINISHED; - return this->status; - } - - if (buf->size == 0) { + if (buf->size <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; diff --git a/src/demuxers/demux_dts.c b/src/demuxers/demux_dts.c index 24a31c1cb..55fc8fb98 100644 --- a/src/demuxers/demux_dts.c +++ b/src/demuxers/demux_dts.c @@ -272,7 +272,7 @@ static int demux_dts_send_chunk (demux_plugin_t *this_gen) { this->frame_size); } - if (buf->size == 0) { + if (buf->size <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; diff --git a/src/demuxers/demux_mpc.c b/src/demuxers/demux_mpc.c index 9b27e5954..220e1b8b6 100644 --- a/src/demuxers/demux_mpc.c +++ b/src/demuxers/demux_mpc.c @@ -209,7 +209,7 @@ static int demux_mpc_send_chunk(demux_plugin_t *this_gen) { /* Read data */ bytes_read = this->input->read(this->input, buf->content, bytes_to_read); - if(bytes_read == 0) { + if(bytes_read <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; diff --git a/src/demuxers/demux_nsf.c b/src/demuxers/demux_nsf.c index 60d5049d9..926ea97e1 100644 --- a/src/demuxers/demux_nsf.c +++ b/src/demuxers/demux_nsf.c @@ -124,7 +124,7 @@ static int demux_nsf_send_chunk(demux_plugin_t *this_gen) { buf->type = BUF_AUDIO_NSF; bytes_read = this->input->read(this->input, buf->content, buf->max_size); - if (bytes_read == 0) { + if (bytes_read <= 0) { /* the file has been completely loaded, free the buffer and start * sending control buffers */ buf->free_buffer(buf); diff --git a/src/demuxers/demux_ogg.c b/src/demuxers/demux_ogg.c index 9e9de45aa..e3a9b20c4 100644 --- a/src/demuxers/demux_ogg.c +++ b/src/demuxers/demux_ogg.c @@ -237,7 +237,7 @@ static int read_ogg_packet (demux_ogg_t *this) { while (ogg_sync_pageout(&this->oy,&this->og)!=1) { buffer = ogg_sync_buffer(&this->oy, CHUNKSIZE); bytes = this->input->read(this->input, buffer, CHUNKSIZE); - if (bytes == 0) { + if (bytes <= 0) { if (total == 0) { lprintf("read_ogg_packet read nothing\n"); return 0; diff --git a/src/demuxers/demux_shn.c b/src/demuxers/demux_shn.c index 4d932305c..ccc34b57f 100644 --- a/src/demuxers/demux_shn.c +++ b/src/demuxers/demux_shn.c @@ -88,7 +88,7 @@ static int demux_shn_send_chunk(demux_plugin_t *this_gen) { buf->pts = 0; bytes_read = this->input->read(this->input, buf->content, buf->max_size); - if (bytes_read == 0) { + if (bytes_read <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; diff --git a/src/demuxers/demux_slave.c b/src/demuxers/demux_slave.c index 28a89a973..abb4d01e5 100644 --- a/src/demuxers/demux_slave.c +++ b/src/demuxers/demux_slave.c @@ -90,10 +90,11 @@ static int demux_slave_next (demux_slave_t *this) { /* fill the scratch buffer */ n = this->input->read(this->input, &this->scratch[this->scratch_used], SCRATCH_SIZE - this->scratch_used); - this->scratch_used += n; + if (n > 0) + this->scratch_used += n; this->scratch[this->scratch_used] = '\0'; - if( !n ) { + if (n <= 0) { lprintf("connection closed\n"); this->status = DEMUX_FINISHED; return 0; diff --git a/src/demuxers/demux_ts.c b/src/demuxers/demux_ts.c index 6c2adc67f..f53a5a3f4 100644 --- a/src/demuxers/demux_ts.c +++ b/src/demuxers/demux_ts.c @@ -1573,7 +1573,7 @@ static unsigned char * demux_synchronise(demux_ts_t* this) { do { read_length = this->input->read(this->input, this->buf, PKT_SIZE * NPKT_PER_READ); - if (read_length % PKT_SIZE) { + if (read_length < 0 || read_length % PKT_SIZE) { xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_ts: read returned %d bytes (not a multiple of %d!)\n", read_length, PKT_SIZE); diff --git a/src/demuxers/demux_tta.c b/src/demuxers/demux_tta.c index 68305a444..de50ac63d 100644 --- a/src/demuxers/demux_tta.c +++ b/src/demuxers/demux_tta.c @@ -126,6 +126,10 @@ static int demux_tta_send_chunk(demux_plugin_t *this_gen) { /* buf->extra_info->input_time = this->current_sample / this->samplerate; */ bytes_read = this->input->read(this->input, buf->content, ( bytes_to_read > buf->max_size ) ? buf->max_size : bytes_to_read); + if (bytes_read < 0) { + this->status = DEMUX_FINISHED; + break; + } buf->size = bytes_read; diff --git a/src/demuxers/demux_vox.c b/src/demuxers/demux_vox.c index d646a756f..1b34106ad 100644 --- a/src/demuxers/demux_vox.c +++ b/src/demuxers/demux_vox.c @@ -77,7 +77,7 @@ static int demux_vox_send_chunk (demux_plugin_t *this_gen) { buf = this->audio_fifo->buffer_pool_alloc (this->audio_fifo); buf->type = BUF_AUDIO_DIALOGIC_IMA; bytes_read = this->input->read(this->input, buf->content, buf->max_size); - if (bytes_read == 0) { + if (bytes_read <= 0) { buf->free_buffer(buf); this->status = DEMUX_FINISHED; return this->status; -- cgit v1.2.3 From bce49846158d839f0fe5185d9956edd1492f9fc3 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 22:36:35 +0100 Subject: check that track's codec_private_len fits in signed variables when decoding matroska while codec_private_len is unsigned, the size is later used to calculate the signed xine_bmiheader.size --- src/demuxers/demux_matroska.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_matroska.c b/src/demuxers/demux_matroska.c index 0a96295ec..12cd282dd 100644 --- a/src/demuxers/demux_matroska.c +++ b/src/demuxers/demux_matroska.c @@ -1302,6 +1302,9 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { xine_bmiheader *bih; lprintf("MATROSKA_CODEC_ID_V_MPEG4_*\n"); + if (track->codec_private_len > 0x7fffffff - sizeof(xine_bmiheader)) + track->codec_private_len = 0x7fffffff - sizeof(xine_bmiheader); + /* create a bitmap info header struct for MPEG 4 */ bih = malloc(sizeof(xine_bmiheader) + track->codec_private_len); bih->biSize = sizeof(xine_bmiheader) + track->codec_private_len; @@ -1323,6 +1326,9 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { xine_bmiheader *bih; lprintf("MATROSKA_CODEC_ID_V_MPEG4_AVC\n"); + if (track->codec_private_len > 0x7fffffff - sizeof(xine_bmiheader)) + track->codec_private_len = 0x7fffffff - sizeof(xine_bmiheader); + /* create a bitmap info header struct for h264 */ bih = malloc(sizeof(xine_bmiheader) + track->codec_private_len); bih->biSize = sizeof(xine_bmiheader) + track->codec_private_len; -- cgit v1.2.3 From 475c79b11056e35dd0716408cd2f1499577b9421 Mon Sep 17 00:00:00 2001 From: Thomas Viehmann Date: Wed, 31 Dec 2008 22:47:32 +0100 Subject: check for negative return values of read when demuxing mng streams Some input plugins (e.g. file) return negative error codes from read, this should be treated as no (more) data available. This is particularly bad because the error code is assigned to an unsigned integer variable for use by the caller. Based on a patch by Matthias Hopf --- src/demuxers/demux_mng.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mng.c b/src/demuxers/demux_mng.c index b57a349c5..d0d83ff80 100644 --- a/src/demuxers/demux_mng.c +++ b/src/demuxers/demux_mng.c @@ -104,7 +104,12 @@ static mng_bool mymng_close_stream(mng_handle mngh){ static mng_bool mymng_read_stream(mng_handle mngh, mng_ptr buffer, mng_uint32 size, mng_uint32 *bytesread){ demux_mng_t *this = (demux_mng_t*)mng_get_userdata(mngh); - *bytesread = this->input->read(this->input, buffer, size); + off_t n = this->input->read(this->input, buffer, size); + if (n < 0) { + *bytesread = 0; + return MNG_FALSE; + } + *bytesread = n; return MNG_TRUE; } -- cgit v1.2.3 From 642887b80beb1f9404f32e68c1f63a11b703841e Mon Sep 17 00:00:00 2001 From: Thomas Viehmann Date: Thu, 1 Jan 2009 01:45:15 +0100 Subject: check for negative/too large return values of get_size when demuxing mod streams get_size might return -1 (e.g. for streams whose size is unknown), but demux_mod is not able to handle this. This is particularly bad because it is later assigned to unsigned types (demux_mod_t.filesize is size_t). Based on a patch by Matthias Hopf . --- src/demuxers/demux_mod.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mod.c b/src/demuxers/demux_mod.c index bffcf36d8..073927707 100644 --- a/src/demuxers/demux_mod.c +++ b/src/demuxers/demux_mod.c @@ -130,9 +130,16 @@ static int probe_mod_file(demux_mod_t *this) { /* returns 1 if the MOD file was opened successfully, 0 otherwise */ static int open_mod_file(demux_mod_t *this) { int total_read; + off_t input_length; /* Get size and create buffer */ - this->filesize = this->input->get_length(this->input); + input_length = this->input->get_length(this->input); + /* Avoid potential issues with signed variables and e.g. read() returning -1 */ + if (input_length > 0x7FFFFFFF || input_length < 0) { + xine_log(this->stream->xine, XINE_LOG_PLUGIN, "modplug - size overflow\n"); + return 0; + } + this->filesize = input_length; this->buffer = (char *)malloc(this->filesize); if(!this->buffer) { xine_log(this->stream->xine, XINE_LOG_PLUGIN, "modplug - allocation failure\n"); -- cgit v1.2.3 From 60ddd2922aabe6704e0baa135ca54e9111782b7c Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 22:57:02 +0100 Subject: handle read errors when demuxing mpeg data Some input plugins (e.g. file) return negative error codes from read, this should be treated as no (more) data available. --- src/demuxers/demux_mpeg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mpeg.c b/src/demuxers/demux_mpeg.c index 388be2140..4be2bfc50 100644 --- a/src/demuxers/demux_mpeg.c +++ b/src/demuxers/demux_mpeg.c @@ -920,7 +920,7 @@ static void demux_mpeg_resync (demux_mpeg_t *this, uint32_t buf) { if (pos == len) { len = this->input->read(this->input, dummy_buf, sizeof(dummy_buf)); pos = 0; - if (len == 0) { + if (len <= 0) { this->status = DEMUX_FINISHED; break; } -- cgit v1.2.3 From 974a1dd501cf1c2db0cb647d9a0ef8010e52b7fe Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 20:47:08 +0100 Subject: abort if buffer for matroska block data cannot be allocated return error when the allocation function returns NULL Otherwise xine might be induced to segfault by bad user data. --- src/demuxers/demux_matroska.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_matroska.c b/src/demuxers/demux_matroska.c index 12cd282dd..924c70bde 100644 --- a/src/demuxers/demux_matroska.c +++ b/src/demuxers/demux_matroska.c @@ -1823,6 +1823,11 @@ static int read_block_data (demux_matroska_t *this, size_t len) { alloc_block_data(this, len); /* block datas */ + if (! this->block_data) { + xprintf(this->stream->xine, XINE_VERBOSITY_LOG, + "demux_matroska: memory allocation error\n"); + return 0; + } if (this->input->read(this->input, this->block_data, len) != len) { off_t pos = this->input->get_current_pos(this->input); xprintf(this->stream->xine, XINE_VERBOSITY_LOG, -- cgit v1.2.3 From 7caa938a4628d487332f4b52c1a571fe8084a9b0 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 22:56:18 +0100 Subject: check return value of input->read_block for NULL in mpeg demuxing --- src/demuxers/demux_mpeg.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mpeg.c b/src/demuxers/demux_mpeg.c index 4be2bfc50..a9dae3040 100644 --- a/src/demuxers/demux_mpeg.c +++ b/src/demuxers/demux_mpeg.c @@ -279,6 +279,10 @@ static void parse_mpeg2_packet (demux_mpeg_t *this, int stream_id, int64_t scr) if((this->dummy_space[0] & 0xE0) == 0x20) { buf = this->input->read_block (this->input, this->video_fifo, len-1); + if (! buf) { + this->status = DEMUX_FINISHED; + return; + } track = (this->dummy_space[0] & 0x1f); @@ -298,6 +302,10 @@ static void parse_mpeg2_packet (demux_mpeg_t *this, int stream_id, int64_t scr) int spu_id = this->dummy_space[1] & 0x03; buf = this->input->read_block (this->input, this->video_fifo, len-1); + if (! buf) { + this->status = DEMUX_FINISHED; + return; + } buf->type = BUF_SPU_SVCD + spu_id; buf->pts = pts; @@ -318,6 +326,10 @@ static void parse_mpeg2_packet (demux_mpeg_t *this, int stream_id, int64_t scr) if((this->dummy_space[0] & 0xfc) == 0x00) { buf = this->input->read_block (this->input, this->video_fifo, len-1); + if (! buf) { + this->status = DEMUX_FINISHED; + return; + } buf->type = BUF_SPU_CVD + (this->dummy_space[0] & 0x03); buf->pts = pts; @@ -376,6 +388,10 @@ static void parse_mpeg2_packet (demux_mpeg_t *this, int stream_id, int64_t scr) i = this->input->read (this->input, this->dummy_space+1, 6); buf = this->input->read_block (this->input, this->video_fifo, len-7); + if (! buf) { + this->status = DEMUX_FINISHED; + return; + } buf->type = BUF_AUDIO_LPCM_BE + track; buf->decoder_flags |= BUF_FLAG_SPECIAL; -- cgit v1.2.3 From afe8c9f28dcde837df80e473c66ef8e4078b3d5d Mon Sep 17 00:00:00 2001 From: Thomas Viehmann Date: Thu, 1 Jan 2009 18:12:40 +0100 Subject: check return value of input->read_block for NULL in yuv_frames demuxing Based on a patch by Matthias Hopf . --- src/demuxers/demux_yuv_frames.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_yuv_frames.c b/src/demuxers/demux_yuv_frames.c index c5ca363ed..089207f58 100644 --- a/src/demuxers/demux_yuv_frames.c +++ b/src/demuxers/demux_yuv_frames.c @@ -126,13 +126,19 @@ static void demux_yuv_frames_send_headers (demux_plugin_t *this_gen){ if(_x_stream_info_get(this->stream, XINE_STREAM_INFO_HAS_AUDIO)) { buf = this->input->read_block(this->input, this->audio_fifo, 0); - this->audio_fifo->put(this->audio_fifo, buf); + if (buf) + this->audio_fifo->put(this->audio_fifo, buf); + else + this->status = DEMUX_FINISHED; } if(_x_stream_info_get(this->stream, XINE_STREAM_INFO_HAS_VIDEO)) { buf = this->input->read_block(this->input, this->video_fifo, 0); - this->video_fifo->put(this->video_fifo, buf); + if (buf) + this->video_fifo->put(this->video_fifo, buf); + else + this->status = DEMUX_FINISHED; } this->status = DEMUX_OK; -- cgit v1.2.3 From 0e71ddedb6098ffc0262fe8225c9b02f5433c9d0 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 22:29:44 +0100 Subject: check size before accessing memory in matroska decoding check the size of allocated buffers to prevent out of bound access --- src/demuxers/demux_matroska.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_matroska.c b/src/demuxers/demux_matroska.c index 924c70bde..f1a423e67 100644 --- a/src/demuxers/demux_matroska.c +++ b/src/demuxers/demux_matroska.c @@ -607,6 +607,8 @@ static void init_codec_xiph(demux_matroska_t *this, matroska_track_t *track) { int i; uint8_t *data; + if (track->codec_private_len < 3) + return; nb_lace = track->codec_private[0]; if (nb_lace != 2) return; @@ -614,6 +616,8 @@ static void init_codec_xiph(demux_matroska_t *this, matroska_track_t *track) { frame[0] = track->codec_private[1]; frame[1] = track->codec_private[2]; frame[2] = track->codec_private_len - frame[0] - frame[1] - 3; + if (frame[2] < 0) + return; data = track->codec_private + 3; for (i = 0; i < 3; i++) { @@ -1288,12 +1292,14 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { if (!strcmp(track->codec_id, MATROSKA_CODEC_ID_V_VFW_FOURCC)) { xine_bmiheader *bih; - lprintf("MATROSKA_CODEC_ID_V_VFW_FOURCC\n"); - bih = (xine_bmiheader*)track->codec_private; - _x_bmiheader_le2me(bih); + if (track->codec_private_len >= sizeof(xine_bmiheader)) { + lprintf("MATROSKA_CODEC_ID_V_VFW_FOURCC\n"); + bih = (xine_bmiheader*)track->codec_private; + _x_bmiheader_le2me(bih); - track->buf_type = _x_fourcc_to_buf_video(bih->biCompression); - init_codec = init_codec_video; + track->buf_type = _x_fourcc_to_buf_video(bih->biCompression); + init_codec = init_codec_video; + } } else if (!strcmp(track->codec_id, MATROSKA_CODEC_ID_V_UNCOMPRESSED)) { } else if ((!strcmp(track->codec_id, MATROSKA_CODEC_ID_V_MPEG4_SP)) || @@ -1405,11 +1411,13 @@ static int parse_track_entry(demux_matroska_t *this, matroska_track_t *track) { xine_waveformatex *wfh; lprintf("MATROSKA_CODEC_ID_A_ACM\n"); - wfh = (xine_waveformatex*)track->codec_private; - _x_waveformatex_le2me(wfh); + if (track->codec_private_len >= sizeof(xine_waveformatex)) { + wfh = (xine_waveformatex*)track->codec_private; + _x_waveformatex_le2me(wfh); - track->buf_type = _x_formattag_to_buf_audio(wfh->wFormatTag); - init_codec = init_codec_audio; + track->buf_type = _x_formattag_to_buf_audio(wfh->wFormatTag); + init_codec = init_codec_audio; + } } else if (!strncmp(track->codec_id, MATROSKA_CODEC_ID_A_AAC, sizeof(MATROSKA_CODEC_ID_A_AAC) - 1)) { lprintf("MATROSKA_CODEC_ID_A_AAC\n"); -- cgit v1.2.3 From eab84e35b652f0d37b2e873d3b894dfe12e4a41b Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Thu, 1 Jan 2009 01:58:17 +0100 Subject: Avoid underflow in input size calculation for compressed atoms if the atom size is shorter than the header size, do not try to decompress anything, as this would lead to zlib reading out of bound data. --- src/demuxers/demux_qt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_qt.c b/src/demuxers/demux_qt.c index c569ef9a4..4ad71e958 100644 --- a/src/demuxers/demux_qt.c +++ b/src/demuxers/demux_qt.c @@ -2208,7 +2208,7 @@ static qt_error open_qt_file(qt_info *info, input_plugin_t *input, } /* check if moov is compressed */ - if (_X_BE_32(&moov_atom[12]) == CMOV_ATOM) { + if (_X_BE_32(&moov_atom[12]) == CMOV_ATOM && moov_atom_size >= 0x28) { info->compressed_header = 1; -- cgit v1.2.3 From 2e77ffc2c7a9498d5c6180871c460a75e5611ac3 Mon Sep 17 00:00:00 2001 From: Thomas Viehmann Date: Thu, 1 Jan 2009 16:57:18 +0100 Subject: check for buffers smaller than headers in real demuxer check buffer lengths to avoid out of bound access when decoding the header. Based on a patch by Matthias Hopf . --- src/demuxers/demux_real.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_real.c b/src/demuxers/demux_real.c index 2b0153450..f2e89afee 100644 --- a/src/demuxers/demux_real.c +++ b/src/demuxers/demux_real.c @@ -318,9 +318,15 @@ static void real_free_mdpr (mdpr_t *mdpr) { } static void real_parse_audio_specific_data (demux_real_t *this, - real_stream_t * stream, - uint8_t * data) + real_stream_t * stream) { + if (stream->mdpr->type_specific_len < 46) { + xprintf (this->stream->xine, XINE_VERBOSITY_LOG, + "demux_real: audio data size smaller than header length!\n"); + return; + } + + uint8_t * data = stream->mdpr->type_specific_data; const uint32_t coded_frame_size = _X_BE_32 (data+24); const uint16_t codec_data_length = _X_BE_16 (data+40); const uint16_t coded_frame_size2 = _X_BE_16 (data+42); @@ -543,11 +549,11 @@ static void real_parse_headers (demux_real_t *this) { this->audio_streams[this->num_audio_streams].mdpr = mdpr; real_parse_audio_specific_data (this, - &this->audio_streams[this->num_audio_streams], - mdpr->type_specific_data); + &this->audio_streams[this->num_audio_streams]); this->num_audio_streams++; - } else if(_X_BE_32(mdpr->type_specific_data + 4) == VIDO_TAG) { + } else if(_X_BE_32(mdpr->type_specific_data + 4) == VIDO_TAG && + mdpr->type_specific_len >= 34) { if(this->num_video_streams == MAX_VIDEO_STREAMS) { xprintf(this->stream->xine, XINE_VERBOSITY_DEBUG, -- cgit v1.2.3 From 4dd616ec236a414c96df5ffeecbb76e077c16571 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 20:09:01 +0100 Subject: handle read errors/insufficient data when forwarding asf data do not forward data if there is not enough --- src/demuxers/demux_asf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_asf.c b/src/demuxers/demux_asf.c index 9d4191633..d79782026 100644 --- a/src/demuxers/demux_asf.c +++ b/src/demuxers/demux_asf.c @@ -738,7 +738,10 @@ static void asf_send_buffer_nodefrag (demux_asf_t *this, asf_demux_stream_t *str bufsize = stream->fifo->buffer_pool_buf_size; buf = stream->fifo->buffer_pool_alloc (stream->fifo); - this->input->read (this->input, buf->content, bufsize); + if (this->input->read (this->input, buf->content, bufsize) != bufsize) { + xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_asf: input buffer starved\n"); + return ; + } lprintf ("data: %d %d %d %d\n", buf->content[0], buf->content[1], buf->content[2], buf->content[3]); @@ -817,7 +820,10 @@ static void asf_send_buffer_defrag (demux_asf_t *this, asf_demux_stream_t *strea if( stream->frag_offset + frag_len > DEFRAG_BUFSIZE ) { xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_asf: buffer overflow on defrag!\n"); } else { - this->input->read (this->input, &stream->buffer[stream->frag_offset], frag_len); + if (this->input->read (this->input, &stream->buffer[stream->frag_offset], frag_len) != frag_len) { + xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_asf: input buffer starved\n"); + return ; + } stream->frag_offset += frag_len; } -- cgit v1.2.3 From 82a5ac7e7ef212e08d63a8f31482548fdb3c9ba4 Mon Sep 17 00:00:00 2001 From: Matthias Hopf Date: Wed, 31 Dec 2008 23:01:54 +0100 Subject: check number of bytes read by input->read in demuxing mpeg block/pes input->read may return negative error codes or read less than we want so we should check for the right return value instead of just not 0 --- src/demuxers/demux_mpeg_block.c | 6 +++--- src/demuxers/demux_mpeg_pes.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src/demuxers') diff --git a/src/demuxers/demux_mpeg_block.c b/src/demuxers/demux_mpeg_block.c index 3ecb88d04..4fa96faeb 100644 --- a/src/demuxers/demux_mpeg_block.c +++ b/src/demuxers/demux_mpeg_block.c @@ -1190,14 +1190,14 @@ static int demux_mpeg_detect_blocksize(demux_mpeg_block_t *this, input_plugin_t *input) { input->seek(input, 2048, SEEK_SET); - if (!input->read(input, this->scratch, 4)) + if (input->read(input, this->scratch, 4) != 4) return 0; if (this->scratch[0] || this->scratch[1] || (this->scratch[2] != 0x01) || (this->scratch[3] != 0xba)) { input->seek(input, 2324, SEEK_SET); - if (!input->read(input, this->scratch, 4)) + if (input->read(input, this->scratch, 4) != 4) return 0; if (this->scratch[0] || this->scratch[1] || (this->scratch[2] != 0x01) || (this->scratch[3] != 0xba)) @@ -1417,7 +1417,7 @@ static demux_plugin_t *open_plugin (demux_class_t *class_gen, xine_stream_t *str } input->seek(input, 0, SEEK_SET); - if (input->read(input, this->scratch, this->blocksize)) { + if (input->read(input, this->scratch, this->blocksize) == this->blocksize) { lprintf("open_plugin:read worked\n"); if (this->scratch[0] || this->scratch[1] diff --git a/src/demuxers/demux_mpeg_pes.c b/src/demuxers/demux_mpeg_pes.c index 32dd2cb51..fd0411e9d 100644 --- a/src/demuxers/demux_mpeg_pes.c +++ b/src/demuxers/demux_mpeg_pes.c @@ -1687,7 +1687,7 @@ static demux_plugin_t *open_plugin (demux_class_t *class_gen, xine_stream_t *str if (((input->get_capabilities(input) & INPUT_CAP_SEEKABLE) != 0) ) { input->seek(input, 0, SEEK_SET); - if (input->read(input, (char *)this->scratch, 6)) { + if (input->read(input, (char *)this->scratch, 6) == 6) { lprintf("open_plugin:read worked\n"); if (this->scratch[0] || this->scratch[1] -- cgit v1.2.3