From 7a29f15cf90a3629185ef1f3987e24a3910785db Mon Sep 17 00:00:00 2001 From: Darren Salt Date: Sun, 23 Mar 2008 01:51:28 +0000 Subject: Check for failure of various memory allocations. (SA29484) Ref. http://aluigi.altervista.org/adv/xinehof-adv.txt --- ChangeLog | 3 ++ src/demuxers/demux_film.c | 5 +++ src/demuxers/demux_flv.c | 16 ++++++-- src/demuxers/demux_qt.c | 85 +++++++++++++++++++++++++------------------ src/demuxers/demux_real.c | 6 ++- src/demuxers/demux_wc3movie.c | 11 +++++- src/demuxers/ebml.c | 5 +++ 7 files changed, 89 insertions(+), 42 deletions(-) diff --git a/ChangeLog b/ChangeLog index 35fd29347..af30be981 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,7 @@ xine-lib (1.1.11.1) 2008-??-?? + * Security fixes: + - Heap overflows in FLV, Qt, Real, WC3Movie, Matroska and FILM demuxers. + * Added a few more memory allocation checks to the above demuxers. * WAV file playback fix: don't assume that the first chunk is "fmt ". xine-lib (1.1.11) 2008-03-19 diff --git a/src/demuxers/demux_film.c b/src/demuxers/demux_film.c index 13036afc1..349c2578f 100644 --- a/src/demuxers/demux_film.c +++ b/src/demuxers/demux_film.c @@ -257,6 +257,8 @@ static int open_film_file(demux_film_t *film) { film->sample_count = _X_BE_32(&film_header[i + 12]); film->sample_table = xine_xmalloc(film->sample_count * sizeof(film_sample_t)); + if (!film->sample_table) + goto film_abort; for (j = 0; j < film->sample_count; j++) { film->sample_table[j].sample_offset = @@ -333,11 +335,14 @@ static int open_film_file(demux_film_t *film) { free(film->interleave_buffer); film->interleave_buffer = xine_xmalloc(film->sample_table[0].sample_size); + if (!film->interleave_buffer) + goto film_abort; } break; default: xine_log(film->stream->xine, XINE_LOG_MSG, _("unrecognized FILM chunk\n")); + film_abort: free (film->interleave_buffer); free (film->sample_table); free (film_header); diff --git a/src/demuxers/demux_flv.c b/src/demuxers/demux_flv.c index a1f0b3a7b..15afe221d 100644 --- a/src/demuxers/demux_flv.c +++ b/src/demuxers/demux_flv.c @@ -85,7 +85,7 @@ typedef struct { off_t filesize; flv_index_entry_t *index; - int num_indices; + unsigned int num_indices; unsigned int cur_pts; @@ -209,7 +209,7 @@ static int parse_flv_var(demux_flv_t *this, unsigned char *end = buf + size; char *str; unsigned char type; - int len, num; + unsigned int len, num; if (size < 1) return 0; @@ -283,6 +283,8 @@ static int parse_flv_var(demux_flv_t *this, str = tmp + 2; tmp += len + 2; len = parse_flv_var(this, tmp, end-tmp, str, len); + if (!len) + return 0; tmp += len; } if (*tmp++ != FLV_DATA_TYPE_ENDOBJECT) @@ -298,6 +300,8 @@ static int parse_flv_var(demux_flv_t *this, str = tmp + 2; tmp += len + 2; len = parse_flv_var(this, tmp, end-tmp, str, len); + if (!len) + return 0; tmp += len; } break; @@ -310,6 +314,8 @@ static int parse_flv_var(demux_flv_t *this, if (this->index) free(this->index); this->index = xine_xmalloc(num*sizeof(flv_index_entry_t)); + if (!this->index) + return 0; this->num_indices = num; } for (num = 0; num < this->num_indices && tmp < end; num++) { @@ -326,6 +332,8 @@ static int parse_flv_var(demux_flv_t *this, if (this->index) free(this->index); this->index = xine_xmalloc(num*sizeof(flv_index_entry_t)); + if (!this->index) + return 0; this->num_indices = num; } for (num = 0; num < this->num_indices && tmp < end; num++) { @@ -339,6 +347,8 @@ static int parse_flv_var(demux_flv_t *this, } while (num-- && tmp < end) { len = parse_flv_var(this, tmp, end-tmp, NULL, 0); + if (!len) + return 0; tmp += len; } break; @@ -360,7 +370,7 @@ static void parse_flv_script(demux_flv_t *this, int size) { unsigned char *end = buf + size; int len; - if (this->input->read(this->input, buf, size ) != size) { + if (!buf || this->input->read(this->input, buf, size ) != size) { this->status = DEMUX_FINISHED; free(buf); return; diff --git a/src/demuxers/demux_qt.c b/src/demuxers/demux_qt.c index a55a0aef3..1be93abd9 100644 --- a/src/demuxers/demux_qt.c +++ b/src/demuxers/demux_qt.c @@ -739,38 +739,52 @@ 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; info->artist = xine_xmalloc(string_size); - strncpy(info->artist, &meta_atom[i + 20], string_size - 1); - info->artist[string_size - 1] = 0; + if (info->artist) { + strncpy(info->artist, &meta_atom[i + 20], string_size - 1); + info->artist[string_size - 1] = 0; + } } else if (current_atom == NAM_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->name = xine_xmalloc(string_size); - strncpy(info->name, &meta_atom[i + 20], string_size - 1); - info->name[string_size - 1] = 0; + if (info->name) { + strncpy(info->name, &meta_atom[i + 20], string_size - 1); + info->name[string_size - 1] = 0; + } } else if (current_atom == ALB_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->album = xine_xmalloc(string_size); - strncpy(info->album, &meta_atom[i + 20], string_size - 1); - info->album[string_size - 1] = 0; + if (info->album) { + strncpy(info->album, &meta_atom[i + 20], string_size - 1); + info->album[string_size - 1] = 0; + } } else if (current_atom == GEN_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->genre = xine_xmalloc(string_size); - strncpy(info->genre, &meta_atom[i + 20], string_size - 1); - info->genre[string_size - 1] = 0; + if (info->genre) { + strncpy(info->genre, &meta_atom[i + 20], string_size - 1); + info->genre[string_size - 1] = 0; + } } else if (current_atom == TOO_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->comment = xine_xmalloc(string_size); - strncpy(info->comment, &meta_atom[i + 20], string_size - 1); - info->comment[string_size - 1] = 0; + if (info->comment) { + strncpy(info->comment, &meta_atom[i + 20], string_size - 1); + info->comment[string_size - 1] = 0; + } } else if (current_atom == WRT_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->composer = xine_xmalloc(string_size); - strncpy(info->composer, &meta_atom[i + 20], string_size - 1); - info->composer[string_size - 1] = 0; + if (info->composer) { + strncpy(info->composer, &meta_atom[i + 20], string_size - 1); + info->composer[string_size - 1] = 0; + } } else if (current_atom == DAY_ATOM) { string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1; info->year = xine_xmalloc(string_size); - strncpy(info->year, &meta_atom[i + 20], string_size - 1); - info->year[string_size - 1] = 0; + if (info->year) { + strncpy(info->year, &meta_atom[i + 20], string_size - 1); + info->year[string_size - 1] = 0; + } } } @@ -1549,32 +1563,29 @@ static qt_error parse_reference_atom (reference_t *ref, current_atom = _X_BE_32(&ref_atom[i]); if (current_atom == RDRF_ATOM) { + 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) + return QT_NOT_A_VALID_FILE; /* if the URL starts with "http://", copy it */ - if (strncmp(&ref_atom[i + 16], "http://", 7) == 0 - || strncmp(&ref_atom[i + 16], "rtsp://", 7) == 0) { + if ( memcmp(&ref_atom[i + 16], "http://", 7) && + memcmp(&ref_atom[i + 16], "rtsp://", 7) && + base_mrl ) + url_offset = strlen(base_mrl); - /* URL is spec'd to terminate with a NULL; don't trust it */ - ref->url = xine_xmalloc(_X_BE_32(&ref_atom[i + 12]) + 1); - strncpy(ref->url, &ref_atom[i + 16], _X_BE_32(&ref_atom[i + 12])); - ref->url[_X_BE_32(&ref_atom[i + 12]) - 1] = '\0'; + /* otherwise, append relative URL to base MRL */ + string_size += url_offset; - } else { + ref->url = xine_xmalloc(string_size + 1); - int string_size; + if ( url_offset ) + strcpy(ref->url, base_mrl); - if (base_mrl) - string_size = strlen(base_mrl) + _X_BE_32(&ref_atom[i + 12]) + 1; - else - string_size = _X_BE_32(&ref_atom[i + 12]) + 1; + memcpy(ref->url + url_offset, &ref_atom[i + 16], _X_BE_32(&ref_atom[i + 12])); - /* otherwise, append relative URL to base MRL */ - ref->url = xine_xmalloc(string_size); - if (base_mrl) - strcpy(ref->url, base_mrl); - strncat(ref->url, &ref_atom[i + 16], _X_BE_32(&ref_atom[i + 12])); - ref->url[string_size - 1] = '\0'; - } + ref->url[string_size] = '\0'; debug_atom_load(" qt rdrf URL reference:\n %s\n", ref->url); @@ -1993,8 +2004,12 @@ static void parse_moov_atom(qt_info *info, unsigned char *moov_atom, info->references = (reference_t *)realloc(info->references, info->reference_count * sizeof(reference_t)); - parse_reference_atom(&info->references[info->reference_count - 1], - &moov_atom[i - 4], info->base_mrl); + error = parse_reference_atom(&info->references[info->reference_count - 1], + &moov_atom[i - 4], info->base_mrl); + if (error != QT_OK) { + info->last_error = error; + return; + } } else { debug_atom_load(" qt: unknown atom into the moov atom (0x%08X)\n", current_atom); diff --git a/src/demuxers/demux_real.c b/src/demuxers/demux_real.c index 9206bfc74..c5096b210 100644 --- a/src/demuxers/demux_real.c +++ b/src/demuxers/demux_real.c @@ -175,7 +175,8 @@ static void real_parse_index(demux_real_t *this) { off_t original_pos = this->input->get_current_pos(this->input); unsigned char index_chunk_header[INDEX_CHUNK_HEADER_SIZE]; unsigned char index_record[INDEX_RECORD_SIZE]; - int i, entries, stream_num; + int i; + unsigned int entries, stream_num; real_index_entry_t **index; while(next_index_chunk) { @@ -230,10 +231,11 @@ static void real_parse_index(demux_real_t *this) { } } - if(index && entries) { + if(index && entries) /* Allocate memory for index */ *index = xine_xmalloc(entries * sizeof(real_index_entry_t)); + if(index && entries && *index) { /* Read index */ for(i = 0; i < entries; i++) { if(this->input->read(this->input, index_record, INDEX_RECORD_SIZE) diff --git a/src/demuxers/demux_wc3movie.c b/src/demuxers/demux_wc3movie.c index 596d47f4a..1a839924d 100644 --- a/src/demuxers/demux_wc3movie.c +++ b/src/demuxers/demux_wc3movie.c @@ -389,6 +389,12 @@ static int open_mve_file(demux_mve_t *this) { /* load the palette chunks */ this->palettes = xine_xmalloc(this->number_of_shots * PALETTE_SIZE * sizeof(palette_entry_t)); + + if (!this->shot_offsets || !this->palettes) { + free (this->shot_offsets); + return 0; + } + for (i = 0; i < this->number_of_shots; i++) { /* make sure there was a valid palette chunk preamble */ if (this->input->read(this->input, preamble, PREAMBLE_SIZE) != @@ -460,8 +466,9 @@ static int open_mve_file(demux_mve_t *this) { case BNAM_TAG: /* load the name into the stream attributes */ - title = realloc (title, chunk_size); - if (this->input->read(this->input, title, chunk_size) != chunk_size) { + free (title); + title = malloc (chunk_size); + if (!title || this->input->read(this->input, title, chunk_size) != chunk_size) { free (title); free (this->palettes); free (this->shot_offsets); diff --git a/src/demuxers/ebml.c b/src/demuxers/ebml.c index ac44aecd7..cc8173c26 100644 --- a/src/demuxers/ebml.c +++ b/src/demuxers/ebml.c @@ -424,10 +424,15 @@ int ebml_check_header(ebml_parser_t *ebml) { case EBML_ID_DOCTYPE: { char *text = malloc(elem.len + 1); + if (!text) + return 0; text[elem.len] = '\0'; if (!ebml_read_ascii (ebml, &elem, text)) + { + free (text); return 0; + } lprintf("doctype: %s\n", text); if (ebml->doctype) -- cgit v1.2.3