From: Josh Coalson Date: Tue, 11 Sep 2007 04:49:56 +0000 (+0000) Subject: extra checking on memory allocation sizes to prevent a class of overflow attacks X-Git-Tag: 1.2.1~55 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0f008d2e9e33205f97d7c7146606e93bb85e2bff;p=platform%2Fupstream%2Fflac.git extra checking on memory allocation sizes to prevent a class of overflow attacks --- diff --git a/src/flac/encode.c b/src/flac/encode.c index e4e0c9c..e14d99b 100644 --- a/src/flac/encode.c +++ b/src/flac/encode.c @@ -40,6 +40,7 @@ #include /* for malloc */ #include /* for strcmp(), strerror() */ #include "FLAC/all.h" +#include "share/alloc.h" #include "share/grabbag.h" #include "encode.h" @@ -1786,10 +1787,10 @@ static void static_metadata_clear(static_metadata_t *m) static FLAC__bool static_metadata_append(static_metadata_t *m, FLAC__StreamMetadata *d, FLAC__bool needs_delete) { void *x; - if(0 == (x = realloc(m->metadata, sizeof(*m->metadata)*(m->num_metadata+1)))) + if(0 == (x = safe_realloc_muladd2_(m->metadata, sizeof(*m->metadata), /*times (*/m->num_metadata, /*+*/1/*)*/))) return false; m->metadata = (FLAC__StreamMetadata**)x; - if(0 == (x = realloc(m->needs_delete, sizeof(*m->needs_delete)*(m->num_metadata+1)))) + if(0 == (x = safe_realloc_muladd2_(m->needs_delete, sizeof(*m->needs_delete), /*times (*/m->num_metadata, /*+*/1/*)*/))) return false; m->needs_delete = (FLAC__bool*)x; m->metadata[m->num_metadata] = d; diff --git a/src/flac/foreign_metadata.c b/src/flac/foreign_metadata.c index c17f9ab..2971912 100644 --- a/src/flac/foreign_metadata.c +++ b/src/flac/foreign_metadata.c @@ -32,6 +32,7 @@ #include /* for memcmp() etc. */ #include "FLAC/assert.h" #include "FLAC/metadata.h" +#include "share/alloc.h" #include "foreign_metadata.h" #ifdef min @@ -73,7 +74,7 @@ static FLAC__bool copy_data_(FILE *fin, FILE *fout, size_t size, const char **er static FLAC__bool append_block_(foreign_metadata_t *fm, off_t offset, FLAC__uint32 size, const char **error) { - foreign_block_t *fb = realloc(fm->blocks, sizeof(foreign_block_t) * (fm->num_blocks+1)); + foreign_block_t *fb = safe_realloc_muladd2_(fm->blocks, sizeof(foreign_block_t), /*times (*/fm->num_blocks, /*+*/1/*)*/); if(fb) { fb[fm->num_blocks].offset = offset; fb[fm->num_blocks].size = size; diff --git a/src/flac/main.c b/src/flac/main.c index 07edf16..a8e4690 100644 --- a/src/flac/main.c +++ b/src/flac/main.c @@ -34,6 +34,7 @@ #include /* for unlink() */ #endif #include "FLAC/all.h" +#include "share/alloc.h" #include "share/grabbag.h" #include "analyze.h" #include "decode.h" @@ -1843,7 +1844,7 @@ int encode_file(const char *infilename, FLAC__bool is_first_file, FLAC__bool is_ if(encode_infile != stdin && grabbag__file_are_same(infilename, outfilename)) { static const char *tmp_suffix = ".tmp,fl-ac+en'c"; /*@@@@ still a remote possibility that a file with this filename exists */ - if(0 == (internal_outfilename = (char *)malloc(strlen(outfilename)+strlen(tmp_suffix)+1))) { + if(0 == (internal_outfilename = (char *)safe_malloc_add_3op_(strlen(outfilename), /*+*/strlen(tmp_suffix), /*+*/1))) { flac__utils_printf(stderr, 1, "ERROR allocating memory for tempfile name\n"); conditional_fclose(encode_infile); return 1; diff --git a/src/libFLAC++/metadata.cpp b/src/libFLAC++/metadata.cpp index f03076d..7039624 100644 --- a/src/libFLAC++/metadata.cpp +++ b/src/libFLAC++/metadata.cpp @@ -29,6 +29,8 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#define __STDC_LIMIT_MACROS 1 /* otherwise SIZE_MAX is not defined for c++ */ +#include "share/alloc.h" #include "FLAC++/metadata.h" #include "FLAC/assert.h" #include // for malloc(), free() @@ -574,7 +576,7 @@ namespace FLAC { clear_entry(); - if(0 == (entry_.entry = (FLAC__byte*)malloc(field_length+1))) { + if(0 == (entry_.entry = (FLAC__byte*)safe_malloc_add_2op_(field_length, /*+*/1))) { is_valid_ = false; } else { @@ -623,7 +625,7 @@ namespace FLAC { clear_field_value(); - if(0 == (field_value_ = (char *)malloc(field_value_length+1))) { + if(0 == (field_value_ = (char *)safe_malloc_add_2op_(field_value_length, /*+*/1))) { is_valid_ = false; } else { @@ -713,7 +715,7 @@ namespace FLAC { { clear_entry(); - if(0 == (entry_.entry = (FLAC__byte*)malloc(field_name_length_ + 1 + field_value_length_ + 1))) { + if(0 == (entry_.entry = (FLAC__byte*)safe_malloc_add_4op_(field_name_length_, /*+*/1, /*+*/field_value_length_, /*+*/1))) { is_valid_ = false; } else { @@ -739,7 +741,7 @@ namespace FLAC { p = (const char *)entry_.entry + entry_.length; field_name_length_ = (unsigned)(p - (const char *)entry_.entry); - if(0 == (field_name_ = (char *)malloc(field_name_length_ + 1))) { // +1 for the trailing \0 + if(0 == (field_name_ = (char *)safe_malloc_add_2op_(field_name_length_, /*+*/1))) { // +1 for the trailing \0 is_valid_ = false; return; } @@ -748,14 +750,14 @@ namespace FLAC { if(entry_.length - field_name_length_ == 0) { field_value_length_ = 0; - if(0 == (field_value_ = (char *)malloc(0))) { + if(0 == (field_value_ = (char *)safe_malloc_(0))) { is_valid_ = false; return; } } else { field_value_length_ = entry_.length - field_name_length_ - 1; - if(0 == (field_value_ = (char *)malloc(field_value_length_ + 1))) { // +1 for the trailing \0 + if(0 == (field_value_ = (char *)safe_malloc_add_2op_(field_value_length_, /*+*/1))) { // +1 for the trailing \0 is_valid_ = false; return; } diff --git a/src/libFLAC/bitwriter.c b/src/libFLAC/bitwriter.c index 4a43530..e3e7ad1 100644 --- a/src/libFLAC/bitwriter.c +++ b/src/libFLAC/bitwriter.c @@ -50,6 +50,7 @@ #include "private/bitwriter.h" #include "private/crc.h" #include "FLAC/assert.h" +#include "share/alloc.h" /* Things should be fastest when this matches the machine word size */ /* WATCHOUT: if you change this you must also change the following #defines down to SWAP_BE_WORD_TO_HOST below to match */ @@ -141,7 +142,7 @@ static FLAC__bool bitwriter_grow_(FLAC__BitWriter *bw, unsigned bits_to_add) FLAC__ASSERT(new_capacity > bw->capacity); FLAC__ASSERT(new_capacity >= bw->words + ((bw->bits + bits_to_add + FLAC__BITS_PER_WORD - 1) / FLAC__BITS_PER_WORD)); - new_buffer = (bwword*)realloc(bw->buffer, sizeof(bwword)*new_capacity); + new_buffer = (bwword*)safe_realloc_mul_2op_(bw->buffer, sizeof(bwword), /*times*/new_capacity); if(new_buffer == 0) return false; bw->buffer = new_buffer; diff --git a/src/libFLAC/include/private/md5.h b/src/libFLAC/include/private/md5.h index 659912c..e5f675a 100644 --- a/src/libFLAC/include/private/md5.h +++ b/src/libFLAC/include/private/md5.h @@ -33,7 +33,7 @@ typedef struct { FLAC__uint32 buf[4]; FLAC__uint32 bytes[2]; FLAC__byte *internal_buf; - unsigned capacity; + size_t capacity; } FLAC__MD5Context; void FLAC__MD5Init(FLAC__MD5Context *context); diff --git a/src/libFLAC/md5.c b/src/libFLAC/md5.c index 47e163f..37cef67 100644 --- a/src/libFLAC/md5.c +++ b/src/libFLAC/md5.c @@ -6,6 +6,7 @@ #include /* for memcpy() */ #include "private/md5.h" +#include "share/alloc.h" #ifndef FLaC__INLINE #define FLaC__INLINE @@ -396,13 +397,19 @@ static void format_input_(FLAC__byte *buf, const FLAC__int32 * const signal[], u */ FLAC__bool FLAC__MD5Accumulate(FLAC__MD5Context *ctx, const FLAC__int32 * const signal[], unsigned channels, unsigned samples, unsigned bytes_per_sample) { - const unsigned bytes_needed = channels * samples * bytes_per_sample; + const size_t bytes_needed = (size_t)channels * (size_t)samples * (size_t)bytes_per_sample; + + /* overflow check */ + if((size_t)channels > SIZE_MAX / (size_t)bytes_per_sample) + return false; + if((size_t)channels * (size_t)bytes_per_sample > SIZE_MAX / (size_t)samples) + return false; if(ctx->capacity < bytes_needed) { FLAC__byte *tmp = (FLAC__byte*)realloc(ctx->internal_buf, bytes_needed); if(0 == tmp) { free(ctx->internal_buf); - if(0 == (ctx->internal_buf = (FLAC__byte*)malloc(bytes_needed))) + if(0 == (ctx->internal_buf = (FLAC__byte*)safe_malloc_(bytes_needed))) return false; } ctx->internal_buf = tmp; diff --git a/src/libFLAC/memory.c b/src/libFLAC/memory.c index 7c7aee9..4d10097 100644 --- a/src/libFLAC/memory.c +++ b/src/libFLAC/memory.c @@ -35,6 +35,7 @@ #include "private/memory.h" #include "FLAC/assert.h" +#include "share/alloc.h" void *FLAC__memory_alloc_aligned(size_t bytes, void **aligned_address) { @@ -44,7 +45,7 @@ void *FLAC__memory_alloc_aligned(size_t bytes, void **aligned_address) #ifdef FLAC__ALIGN_MALLOC_DATA /* align on 32-byte (256-bit) boundary */ - x = malloc(bytes+31); + x = safe_malloc_add_2op_(bytes, /*+*/31); #ifdef SIZEOF_VOIDP #if SIZEOF_VOIDP == 4 /* could do *aligned_address = x + ((unsigned) (32 - (((unsigned)x) & 31))) & 31; */ @@ -64,7 +65,7 @@ void *FLAC__memory_alloc_aligned(size_t bytes, void **aligned_address) return 0; #endif #else - x = malloc(bytes); + x = safe_malloc_(bytes); *aligned_address = x; #endif return x; @@ -83,7 +84,10 @@ FLAC__bool FLAC__memory_alloc_aligned_int32_array(unsigned elements, FLAC__int32 FLAC__ASSERT(0 != aligned_pointer); FLAC__ASSERT(unaligned_pointer != aligned_pointer); - pu = (FLAC__int32*)FLAC__memory_alloc_aligned(sizeof(FLAC__int32) * elements, &u.pv); + if((size_t)elements > SIZE_MAX / sizeof(*pu)) /* overflow check */ + return false; + + pu = (FLAC__int32*)FLAC__memory_alloc_aligned(sizeof(*pu) * (size_t)elements, &u.pv); if(0 == pu) { return false; } @@ -109,7 +113,10 @@ FLAC__bool FLAC__memory_alloc_aligned_uint32_array(unsigned elements, FLAC__uint FLAC__ASSERT(0 != aligned_pointer); FLAC__ASSERT(unaligned_pointer != aligned_pointer); - pu = (FLAC__uint32*)FLAC__memory_alloc_aligned(sizeof(FLAC__uint32) * elements, &u.pv); + if((size_t)elements > SIZE_MAX / sizeof(*pu)) /* overflow check */ + return false; + + pu = (FLAC__uint32*)FLAC__memory_alloc_aligned(sizeof(*pu) * elements, &u.pv); if(0 == pu) { return false; } @@ -135,7 +142,10 @@ FLAC__bool FLAC__memory_alloc_aligned_uint64_array(unsigned elements, FLAC__uint FLAC__ASSERT(0 != aligned_pointer); FLAC__ASSERT(unaligned_pointer != aligned_pointer); - pu = (FLAC__uint64*)FLAC__memory_alloc_aligned(sizeof(FLAC__uint64) * elements, &u.pv); + if((size_t)elements > SIZE_MAX / sizeof(*pu)) /* overflow check */ + return false; + + pu = (FLAC__uint64*)FLAC__memory_alloc_aligned(sizeof(*pu) * elements, &u.pv); if(0 == pu) { return false; } @@ -161,7 +171,10 @@ FLAC__bool FLAC__memory_alloc_aligned_unsigned_array(unsigned elements, unsigned FLAC__ASSERT(0 != aligned_pointer); FLAC__ASSERT(unaligned_pointer != aligned_pointer); - pu = (unsigned*)FLAC__memory_alloc_aligned(sizeof(unsigned) * elements, &u.pv); + if((size_t)elements > SIZE_MAX / sizeof(*pu)) /* overflow check */ + return false; + + pu = (unsigned*)FLAC__memory_alloc_aligned(sizeof(*pu) * elements, &u.pv); if(0 == pu) { return false; } @@ -189,7 +202,10 @@ FLAC__bool FLAC__memory_alloc_aligned_real_array(unsigned elements, FLAC__real * FLAC__ASSERT(0 != aligned_pointer); FLAC__ASSERT(unaligned_pointer != aligned_pointer); - pu = (FLAC__real*)FLAC__memory_alloc_aligned(sizeof(FLAC__real) * elements, &u.pv); + if((size_t)elements > SIZE_MAX / sizeof(*pu)) /* overflow check */ + return false; + + pu = (FLAC__real*)FLAC__memory_alloc_aligned(sizeof(*pu) * elements, &u.pv); if(0 == pu) { return false; } diff --git a/src/libFLAC/metadata_iterators.c b/src/libFLAC/metadata_iterators.c index 32eef2f..66ffdc3 100644 --- a/src/libFLAC/metadata_iterators.c +++ b/src/libFLAC/metadata_iterators.c @@ -61,6 +61,7 @@ #include "FLAC/assert.h" #include "FLAC/stream_decoder.h" +#include "share/alloc.h" #ifdef max #undef max @@ -2152,6 +2153,9 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_application_cb_(FLA if(read_cb(block->id, 1, id_bytes, handle) != id_bytes) return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_READ_ERROR; + if(block_length < id_bytes) + return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_READ_ERROR; + block_length -= id_bytes; if(block_length == 0) { @@ -2179,7 +2183,7 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC_ if(block->num_points == 0) block->points = 0; - else if(0 == (block->points = (FLAC__StreamMetadata_SeekPoint*)malloc(block->num_points * sizeof(FLAC__StreamMetadata_SeekPoint)))) + else if(0 == (block->points = (FLAC__StreamMetadata_SeekPoint*)safe_malloc_mul_2op_(block->num_points, /*times*/sizeof(FLAC__StreamMetadata_SeekPoint)))) return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR; for(i = 0; i < block->num_points; i++) { @@ -2212,7 +2216,7 @@ FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_vorbis_comment_entr entry->entry = 0; } else { - if(0 == (entry->entry = (FLAC__byte*)malloc(entry->length+1))) + if(0 == (entry->entry = (FLAC__byte*)safe_malloc_add_2op_(entry->length, /*+*/1))) return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR; if(read_cb(entry->entry, 1, entry->length, handle) != entry->length) @@ -2387,7 +2391,7 @@ static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_picture_cstr if(0 != *data) free(*data); - if(0 == (*data = (FLAC__byte*)malloc(*length+1))) + if(0 == (*data = (FLAC__byte*)safe_malloc_add_2op_(*length, /*+*/1))) return FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR; if(*length > 0) { @@ -3206,7 +3210,7 @@ FLAC__bool open_tempfile_(const char *filename, const char *tempfile_path_prefix { static const char *tempfile_suffix = ".metadata_edit"; if(0 == tempfile_path_prefix) { - if(0 == (*tempfilename = (char*)malloc(strlen(filename) + strlen(tempfile_suffix) + 1))) { + if(0 == (*tempfilename = (char*)safe_malloc_add_3op_(strlen(filename), /*+*/strlen(tempfile_suffix), /*+*/1))) { *status = FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR; return false; } @@ -3220,7 +3224,7 @@ FLAC__bool open_tempfile_(const char *filename, const char *tempfile_path_prefix else p++; - if(0 == (*tempfilename = (char*)malloc(strlen(tempfile_path_prefix) + 1 + strlen(p) + strlen(tempfile_suffix) + 1))) { + if(0 == (*tempfilename = (char*)safe_malloc_add_4op_(strlen(tempfile_path_prefix), /*+*/strlen(p), /*+*/strlen(tempfile_suffix), /*+*/2))) { *status = FLAC__METADATA_SIMPLE_ITERATOR_STATUS_MEMORY_ALLOCATION_ERROR; return false; } diff --git a/src/libFLAC/metadata_object.c b/src/libFLAC/metadata_object.c index 3cfe89c..9a6b5d2 100644 --- a/src/libFLAC/metadata_object.c +++ b/src/libFLAC/metadata_object.c @@ -39,6 +39,7 @@ #include "private/metadata.h" #include "FLAC/assert.h" +#include "share/alloc.h" /**************************************************************************** @@ -53,14 +54,14 @@ * from != NULL && bytes > 0 * to <- copy of from * else ASSERT - * malloc error leaved 'to' unchanged + * malloc error leaves 'to' unchanged */ static FLAC__bool copy_bytes_(FLAC__byte **to, const FLAC__byte *from, unsigned bytes) { FLAC__ASSERT(0 != to); if(bytes > 0 && 0 != from) { FLAC__byte *x; - if(0 == (x = (FLAC__byte*)malloc(bytes))) + if(0 == (x = (FLAC__byte*)safe_malloc_(bytes))) return false; memcpy(x, from, bytes); *to = x; @@ -94,7 +95,7 @@ static FLAC__bool free_copy_bytes_(FLAC__byte **to, const FLAC__byte *from, unsi /* realloc() failure leaves entry unchanged */ static FLAC__bool ensure_null_terminated_(FLAC__byte **entry, unsigned length) { - FLAC__byte *x = (FLAC__byte*)realloc(*entry, length+1); + FLAC__byte *x = (FLAC__byte*)safe_realloc_add_2op_(*entry, length, /*+*/1); if(0 != x) { x[length] = '\0'; *entry = x; @@ -132,7 +133,7 @@ static FLAC__bool copy_vcentry_(FLAC__StreamMetadata_VorbisComment_Entry *to, co else { FLAC__byte *x; FLAC__ASSERT(from->length > 0); - if(0 == (x = (FLAC__byte*)malloc(from->length+1))) + if(0 == (x = (FLAC__byte*)safe_malloc_add_2op_(from->length, /*+*/1))) return false; memcpy(x, from->entry, from->length); x[from->length] = '\0'; @@ -150,7 +151,7 @@ static FLAC__bool copy_track_(FLAC__StreamMetadata_CueSheet_Track *to, const FLA else { FLAC__StreamMetadata_CueSheet_Index *x; FLAC__ASSERT(from->num_indices > 0); - if(0 == (x = (FLAC__StreamMetadata_CueSheet_Index*)malloc(from->num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index)))) + if(0 == (x = (FLAC__StreamMetadata_CueSheet_Index*)safe_malloc_mul_2op_(from->num_indices, /*times*/sizeof(FLAC__StreamMetadata_CueSheet_Index)))) return false; memcpy(x, from->indices, from->num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index)); to->indices = x; @@ -172,7 +173,7 @@ static FLAC__StreamMetadata_SeekPoint *seekpoint_array_new_(unsigned num_points) FLAC__ASSERT(num_points > 0); - object_array = (FLAC__StreamMetadata_SeekPoint*)malloc(num_points * sizeof(FLAC__StreamMetadata_SeekPoint)); + object_array = (FLAC__StreamMetadata_SeekPoint*)safe_malloc_mul_2op_(num_points, /*times*/sizeof(FLAC__StreamMetadata_SeekPoint)); if(0 != object_array) { unsigned i; @@ -205,7 +206,7 @@ static FLAC__StreamMetadata_VorbisComment_Entry *vorbiscomment_entry_array_new_( { FLAC__ASSERT(num_comments > 0); - return (FLAC__StreamMetadata_VorbisComment_Entry*)calloc(num_comments, sizeof(FLAC__StreamMetadata_VorbisComment_Entry)); + return (FLAC__StreamMetadata_VorbisComment_Entry*)safe_calloc_(num_comments, sizeof(FLAC__StreamMetadata_VorbisComment_Entry)); } static void vorbiscomment_entry_array_delete_(FLAC__StreamMetadata_VorbisComment_Entry *object_array, unsigned num_comments) @@ -344,14 +345,14 @@ static FLAC__StreamMetadata_CueSheet_Index *cuesheet_track_index_array_new_(unsi { FLAC__ASSERT(num_indices > 0); - return (FLAC__StreamMetadata_CueSheet_Index*)calloc(num_indices, sizeof(FLAC__StreamMetadata_CueSheet_Index)); + return (FLAC__StreamMetadata_CueSheet_Index*)safe_calloc_(num_indices, sizeof(FLAC__StreamMetadata_CueSheet_Index)); } static FLAC__StreamMetadata_CueSheet_Track *cuesheet_track_array_new_(unsigned num_tracks) { FLAC__ASSERT(num_tracks > 0); - return (FLAC__StreamMetadata_CueSheet_Track*)calloc(num_tracks, sizeof(FLAC__StreamMetadata_CueSheet_Track)); + return (FLAC__StreamMetadata_CueSheet_Track*)safe_calloc_(num_tracks, sizeof(FLAC__StreamMetadata_CueSheet_Track)); } static void cuesheet_track_array_delete_(FLAC__StreamMetadata_CueSheet_Track *object_array, unsigned num_tracks) @@ -537,6 +538,10 @@ FLAC_API FLAC__StreamMetadata *FLAC__metadata_object_clone(const FLAC__StreamMet case FLAC__METADATA_TYPE_PADDING: break; case FLAC__METADATA_TYPE_APPLICATION: + if(to->length < FLAC__STREAM_METADATA_APPLICATION_ID_LEN / 8) { /* underflow check */ + FLAC__metadata_object_delete(to); + return 0; + } memcpy(&to->data.application.id, &object->data.application.id, FLAC__STREAM_METADATA_APPLICATION_ID_LEN / 8); if(!copy_bytes_(&to->data.application.data, object->data.application.data, object->length - FLAC__STREAM_METADATA_APPLICATION_ID_LEN / 8)) { FLAC__metadata_object_delete(to); @@ -545,6 +550,10 @@ FLAC_API FLAC__StreamMetadata *FLAC__metadata_object_clone(const FLAC__StreamMet break; case FLAC__METADATA_TYPE_SEEKTABLE: to->data.seek_table.num_points = object->data.seek_table.num_points; + if(to->data.seek_table.num_points > SIZE_MAX / sizeof(FLAC__StreamMetadata_SeekPoint)) { /* overflow check */ + FLAC__metadata_object_delete(to); + return 0; + } if(!copy_bytes_((FLAC__byte**)&to->data.seek_table.points, (FLAC__byte*)object->data.seek_table.points, object->data.seek_table.num_points * sizeof(FLAC__StreamMetadata_SeekPoint))) { FLAC__metadata_object_delete(to); return 0; @@ -930,8 +939,12 @@ FLAC_API FLAC__bool FLAC__metadata_object_seektable_resize_points(FLAC__StreamMe return false; } else { - const unsigned old_size = object->data.seek_table.num_points * sizeof(FLAC__StreamMetadata_SeekPoint); - const unsigned new_size = new_num_points * sizeof(FLAC__StreamMetadata_SeekPoint); + const size_t old_size = object->data.seek_table.num_points * sizeof(FLAC__StreamMetadata_SeekPoint); + const size_t new_size = new_num_points * sizeof(FLAC__StreamMetadata_SeekPoint); + + /* overflow check */ + if((size_t)new_num_points > SIZE_MAX / sizeof(FLAC__StreamMetadata_SeekPoint)) + return false; FLAC__ASSERT(object->data.seek_table.num_points > 0); @@ -1157,8 +1170,12 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_resize_comments(FLAC__St return false; } else { - const unsigned old_size = object->data.vorbis_comment.num_comments * sizeof(FLAC__StreamMetadata_VorbisComment_Entry); - const unsigned new_size = new_num_comments * sizeof(FLAC__StreamMetadata_VorbisComment_Entry); + const size_t old_size = object->data.vorbis_comment.num_comments * sizeof(FLAC__StreamMetadata_VorbisComment_Entry); + const size_t new_size = new_num_comments * sizeof(FLAC__StreamMetadata_VorbisComment_Entry); + + /* overflow check */ + if((size_t)new_num_comments > SIZE_MAX / sizeof(FLAC__StreamMetadata_VorbisComment_Entry)) + return false; FLAC__ASSERT(object->data.vorbis_comment.num_comments > 0); @@ -1306,7 +1323,7 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_from_name_value_pa const size_t nn = strlen(field_name); const size_t nv = strlen(field_value); entry->length = nn + 1 /*=*/ + nv; - if(0 == (entry->entry = (FLAC__byte*)malloc(entry->length+1))) + if(0 == (entry->entry = (FLAC__byte*)safe_malloc_add_4op_(nn, /*+*/1, /*+*/nv, /*+*/1))) return false; memcpy(entry->entry, field_name, nn); entry->entry[nn] = '='; @@ -1333,9 +1350,9 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_to_name_value_pair FLAC__ASSERT(0 != eq); if(0 == eq) return false; /* double protection */ - if(0 == (*field_name = (char*)malloc(nn+1))) + if(0 == (*field_name = (char*)safe_malloc_add_2op_(nn, /*+*/1))) return false; - if(0 == (*field_value = (char*)malloc(nv+1))) { + if(0 == (*field_value = (char*)safe_malloc_add_2op_(nv, /*+*/1))) { free(*field_name); return false; } @@ -1465,8 +1482,12 @@ FLAC_API FLAC__bool FLAC__metadata_object_cuesheet_track_resize_indices(FLAC__St return false; } else { - const unsigned old_size = track->num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index); - const unsigned new_size = new_num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index); + const size_t old_size = track->num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index); + const size_t new_size = new_num_indices * sizeof(FLAC__StreamMetadata_CueSheet_Index); + + /* overflow check */ + if((size_t)new_num_indices > SIZE_MAX / sizeof(FLAC__StreamMetadata_CueSheet_Index)) + return false; FLAC__ASSERT(track->num_indices > 0); @@ -1549,8 +1570,12 @@ FLAC_API FLAC__bool FLAC__metadata_object_cuesheet_resize_tracks(FLAC__StreamMet return false; } else { - const unsigned old_size = object->data.cue_sheet.num_tracks * sizeof(FLAC__StreamMetadata_CueSheet_Track); - const unsigned new_size = new_num_tracks * sizeof(FLAC__StreamMetadata_CueSheet_Track); + const size_t old_size = object->data.cue_sheet.num_tracks * sizeof(FLAC__StreamMetadata_CueSheet_Track); + const size_t new_size = new_num_tracks * sizeof(FLAC__StreamMetadata_CueSheet_Track); + + /* overflow check */ + if((size_t)new_num_tracks > SIZE_MAX / sizeof(FLAC__StreamMetadata_CueSheet_Track)) + return false; FLAC__ASSERT(object->data.cue_sheet.num_tracks > 0); @@ -1707,6 +1732,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_picture_set_mime_type(FLAC__StreamMeta /* do the copy first so that if we fail we leave the object untouched */ if(copy) { + if(new_length >= SIZE_MAX) /* overflow check */ + return false; if(!copy_bytes_((FLAC__byte**)(&object->data.picture.mime_type), (FLAC__byte*)mime_type, new_length+1)) return false; } @@ -1737,6 +1764,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_picture_set_description(FLAC__StreamMe /* do the copy first so that if we fail we leave the object untouched */ if(copy) { + if(new_length >= SIZE_MAX) /* overflow check */ + return false; if(!copy_bytes_(&object->data.picture.description, description, new_length+1)) return false; } diff --git a/src/libFLAC/ogg_helper.c b/src/libFLAC/ogg_helper.c index aeaf99f..73f9f0b 100644 --- a/src/libFLAC/ogg_helper.c +++ b/src/libFLAC/ogg_helper.c @@ -36,6 +36,7 @@ #include /* for malloc() */ #include /* for memcmp(), memcpy() */ #include "FLAC/assert.h" +#include "share/alloc.h" #include "private/ogg_helper.h" #include "protected/stream_encoder.h" @@ -112,7 +113,7 @@ FLAC__bool simple_ogg_page__get_at(FLAC__StreamEncoder *encoder, FLAC__uint64 po } /* allocate space for the page header */ - if(0 == (page->header = (unsigned char *)malloc(OGG_MAX_HEADER_LEN))) { + if(0 == (page->header = (unsigned char *)safe_malloc_(OGG_MAX_HEADER_LEN))) { encoder->protected_->state = FLAC__STREAM_ENCODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -154,7 +155,7 @@ FLAC__bool simple_ogg_page__get_at(FLAC__StreamEncoder *encoder, FLAC__uint64 po } /* allocate space for the page body */ - if(0 == (page->body = (unsigned char *)malloc(page->body_len))) { + if(0 == (page->body = (unsigned char *)safe_malloc_(page->body_len))) { encoder->protected_->state = FLAC__STREAM_ENCODER_MEMORY_ALLOCATION_ERROR; return false; } diff --git a/src/libFLAC/stream_decoder.c b/src/libFLAC/stream_decoder.c index 505958f..e368513 100644 --- a/src/libFLAC/stream_decoder.c +++ b/src/libFLAC/stream_decoder.c @@ -53,6 +53,7 @@ #endif #endif #include "FLAC/assert.h" +#include "share/alloc.h" #include "protected/stream_decoder.h" #include "private/bitreader.h" #include "private/bitmath.h" @@ -181,7 +182,7 @@ typedef struct FLAC__StreamDecoderPrivate { FLAC__StreamMetadata seek_table; FLAC__bool metadata_filter[128]; /* MAGIC number 128 == total number of metadata block types == 1 << 7 */ FLAC__byte *metadata_filter_ids; - unsigned metadata_filter_ids_count, metadata_filter_ids_capacity; /* units for both are IDs, not bytes */ + size_t metadata_filter_ids_count, metadata_filter_ids_capacity; /* units for both are IDs, not bytes */ FLAC__Frame frame; FLAC__bool cached; /* true if there is a byte in lookahead */ FLAC__CPUInfo cpuinfo; @@ -784,7 +785,7 @@ FLAC_API FLAC__bool FLAC__stream_decoder_set_metadata_respond_application(FLAC__ FLAC__ASSERT(0 != decoder->private_->metadata_filter_ids); if(decoder->private_->metadata_filter_ids_count == decoder->private_->metadata_filter_ids_capacity) { - if(0 == (decoder->private_->metadata_filter_ids = (FLAC__byte*)realloc(decoder->private_->metadata_filter_ids, decoder->private_->metadata_filter_ids_capacity * 2))) { + if(0 == (decoder->private_->metadata_filter_ids = (FLAC__byte*)safe_realloc_mul_2op_(decoder->private_->metadata_filter_ids, decoder->private_->metadata_filter_ids_capacity, /*times*/2))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -843,7 +844,7 @@ FLAC_API FLAC__bool FLAC__stream_decoder_set_metadata_ignore_application(FLAC__S FLAC__ASSERT(0 != decoder->private_->metadata_filter_ids); if(decoder->private_->metadata_filter_ids_count == decoder->private_->metadata_filter_ids_capacity) { - if(0 == (decoder->private_->metadata_filter_ids = (FLAC__byte*)realloc(decoder->private_->metadata_filter_ids, decoder->private_->metadata_filter_ids_capacity * 2))) { + if(0 == (decoder->private_->metadata_filter_ids = (FLAC__byte*)safe_realloc_mul_2op_(decoder->private_->metadata_filter_ids, decoder->private_->metadata_filter_ids_capacity, /*times*/2))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1325,7 +1326,7 @@ FLAC__bool allocate_output_(FLAC__StreamDecoder *decoder, unsigned size, unsigne * (at negative indices) for alignment purposes; we use 4 * to keep the data well-aligned. */ - tmp = (FLAC__int32*)malloc(sizeof(FLAC__int32)*(size+4)); + tmp = (FLAC__int32*)safe_malloc_muladd2_(sizeof(FLAC__int32), /*times (*/size, /*+*/4/*)*/); if(tmp == 0) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; @@ -1350,7 +1351,7 @@ FLAC__bool allocate_output_(FLAC__StreamDecoder *decoder, unsigned size, unsigne FLAC__bool has_id_filtered_(FLAC__StreamDecoder *decoder, FLAC__byte *id) { - unsigned i; + size_t i; FLAC__ASSERT(0 != decoder); FLAC__ASSERT(0 != decoder->private_); @@ -1471,6 +1472,11 @@ FLAC__bool read_metadata_(FLAC__StreamDecoder *decoder) if(!FLAC__bitreader_read_byte_block_aligned_no_crc(decoder->private_->input, block.data.application.id, FLAC__STREAM_METADATA_APPLICATION_ID_LEN/8)) return false; /* read_callback_ sets the state for us */ + if(real_length < FLAC__STREAM_METADATA_APPLICATION_ID_LEN/8) { /* underflow check */ + decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;/*@@@@@@ maybe wrong error? need to resync?*/ + return false; + } + real_length -= FLAC__STREAM_METADATA_APPLICATION_ID_LEN/8; if(decoder->private_->metadata_filter_ids_count > 0 && has_id_filtered_(decoder, block.data.application.id)) @@ -1533,7 +1539,7 @@ FLAC__bool read_metadata_(FLAC__StreamDecoder *decoder) if(!decoder->private_->is_seeking && decoder->private_->metadata_callback) decoder->private_->metadata_callback(decoder, &block, decoder->private_->client_data); - /* now we have to free any malloc'ed data in the block */ + /* now we have to free any malloc()ed data in the block */ switch(type) { case FLAC__METADATA_TYPE_PADDING: break; @@ -1673,7 +1679,7 @@ FLAC__bool read_metadata_seektable_(FLAC__StreamDecoder *decoder, FLAC__bool is_ decoder->private_->seek_table.data.seek_table.num_points = length / FLAC__STREAM_METADATA_SEEKPOINT_LENGTH; /* use realloc since we may pass through here several times (e.g. after seeking) */ - if(0 == (decoder->private_->seek_table.data.seek_table.points = (FLAC__StreamMetadata_SeekPoint*)realloc(decoder->private_->seek_table.data.seek_table.points, decoder->private_->seek_table.data.seek_table.num_points * sizeof(FLAC__StreamMetadata_SeekPoint)))) { + if(0 == (decoder->private_->seek_table.data.seek_table.points = (FLAC__StreamMetadata_SeekPoint*)safe_realloc_mul_2op_(decoder->private_->seek_table.data.seek_table.points, decoder->private_->seek_table.data.seek_table.num_points, /*times*/sizeof(FLAC__StreamMetadata_SeekPoint)))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1712,7 +1718,7 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->vendor_string.length)) return false; /* read_callback_ sets the state for us */ if(obj->vendor_string.length > 0) { - if(0 == (obj->vendor_string.entry = (FLAC__byte*)malloc(obj->vendor_string.length+1))) { + if(0 == (obj->vendor_string.entry = (FLAC__byte*)safe_malloc_add_2op_(obj->vendor_string.length, /*+*/1))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1730,7 +1736,7 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre /* read comments */ if(obj->num_comments > 0) { - if(0 == (obj->comments = (FLAC__StreamMetadata_VorbisComment_Entry*)malloc(obj->num_comments * sizeof(FLAC__StreamMetadata_VorbisComment_Entry)))) { + if(0 == (obj->comments = (FLAC__StreamMetadata_VorbisComment_Entry*)safe_malloc_mul_2op_(obj->num_comments, /*times*/sizeof(FLAC__StreamMetadata_VorbisComment_Entry)))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1739,7 +1745,7 @@ FLAC__bool read_metadata_vorbiscomment_(FLAC__StreamDecoder *decoder, FLAC__Stre if(!FLAC__bitreader_read_uint32_little_endian(decoder->private_->input, &obj->comments[i].length)) return false; /* read_callback_ sets the state for us */ if(obj->comments[i].length > 0) { - if(0 == (obj->comments[i].entry = (FLAC__byte*)malloc(obj->comments[i].length+1))) { + if(0 == (obj->comments[i].entry = (FLAC__byte*)safe_malloc_add_2op_(obj->comments[i].length, /*+*/1))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1785,7 +1791,7 @@ FLAC__bool read_metadata_cuesheet_(FLAC__StreamDecoder *decoder, FLAC__StreamMet obj->num_tracks = x; if(obj->num_tracks > 0) { - if(0 == (obj->tracks = (FLAC__StreamMetadata_CueSheet_Track*)calloc(obj->num_tracks, sizeof(FLAC__StreamMetadata_CueSheet_Track)))) { + if(0 == (obj->tracks = (FLAC__StreamMetadata_CueSheet_Track*)safe_calloc_(obj->num_tracks, sizeof(FLAC__StreamMetadata_CueSheet_Track)))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1818,7 +1824,7 @@ FLAC__bool read_metadata_cuesheet_(FLAC__StreamDecoder *decoder, FLAC__StreamMet track->num_indices = (FLAC__byte)x; if(track->num_indices > 0) { - if(0 == (track->indices = (FLAC__StreamMetadata_CueSheet_Index*)calloc(track->num_indices, sizeof(FLAC__StreamMetadata_CueSheet_Index)))) { + if(0 == (track->indices = (FLAC__StreamMetadata_CueSheet_Index*)safe_calloc_(track->num_indices, sizeof(FLAC__StreamMetadata_CueSheet_Index)))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1855,7 +1861,7 @@ FLAC__bool read_metadata_picture_(FLAC__StreamDecoder *decoder, FLAC__StreamMeta /* read MIME type */ if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &x, FLAC__STREAM_METADATA_PICTURE_MIME_TYPE_LENGTH_LEN)) return false; /* read_callback_ sets the state for us */ - if(0 == (obj->mime_type = (char*)malloc(x+1))) { + if(0 == (obj->mime_type = (char*)safe_malloc_add_2op_(x, /*+*/1))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1868,7 +1874,7 @@ FLAC__bool read_metadata_picture_(FLAC__StreamDecoder *decoder, FLAC__StreamMeta /* read description */ if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &x, FLAC__STREAM_METADATA_PICTURE_DESCRIPTION_LENGTH_LEN)) return false; /* read_callback_ sets the state for us */ - if(0 == (obj->description = (FLAC__byte*)malloc(x+1))) { + if(0 == (obj->description = (FLAC__byte*)safe_malloc_add_2op_(x, /*+*/1))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } @@ -1897,7 +1903,7 @@ FLAC__bool read_metadata_picture_(FLAC__StreamDecoder *decoder, FLAC__StreamMeta /* read data */ if(!FLAC__bitreader_read_raw_uint32(decoder->private_->input, &(obj->data_length), FLAC__STREAM_METADATA_PICTURE_DATA_LENGTH_LEN)) return false; /* read_callback_ sets the state for us */ - if(0 == (obj->data = (FLAC__byte*)malloc(obj->data_length))) { + if(0 == (obj->data = (FLAC__byte*)safe_malloc_(obj->data_length))) { decoder->protected_->state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR; return false; } diff --git a/src/libFLAC/stream_encoder.c b/src/libFLAC/stream_encoder.c index 85109cf..04c2a29 100644 --- a/src/libFLAC/stream_encoder.c +++ b/src/libFLAC/stream_encoder.c @@ -54,6 +54,7 @@ #endif #include "FLAC/assert.h" #include "FLAC/stream_decoder.h" +#include "share/alloc.h" #include "protected/stream_encoder.h" #include "private/bitwriter.h" #include "private/bitmath.h" @@ -993,7 +994,7 @@ static FLAC__StreamEncoderInitStatus init_stream_internal_( */ encoder->private_->verify.input_fifo.size = encoder->protected_->blocksize+OVERREAD_; for(i = 0; i < encoder->protected_->channels; i++) { - if(0 == (encoder->private_->verify.input_fifo.data[i] = (FLAC__int32*)malloc(sizeof(FLAC__int32) * encoder->private_->verify.input_fifo.size))) { + if(0 == (encoder->private_->verify.input_fifo.data[i] = (FLAC__int32*)safe_malloc_mul_2op_(sizeof(FLAC__int32), /*times*/encoder->private_->verify.input_fifo.size))) { encoder->protected_->state = FLAC__STREAM_ENCODER_MEMORY_ALLOCATION_ERROR; return FLAC__STREAM_ENCODER_INIT_STATUS_ENCODER_ERROR; } @@ -1731,7 +1732,7 @@ FLAC_API FLAC__bool FLAC__stream_encoder_set_metadata(FLAC__StreamEncoder *encod } if(num_blocks) { FLAC__StreamMetadata **m; - if(0 == (m = (FLAC__StreamMetadata**)malloc(sizeof(m[0]) * num_blocks))) + if(0 == (m = (FLAC__StreamMetadata**)safe_malloc_mul_2op_(sizeof(m[0]), /*times*/num_blocks))) return false; memcpy(m, metadata, sizeof(m[0]) * num_blocks); encoder->protected_->metadata = m; diff --git a/src/metaflac/operations.c b/src/metaflac/operations.c index e905b76..54774bf 100644 --- a/src/metaflac/operations.c +++ b/src/metaflac/operations.c @@ -25,6 +25,7 @@ #include "utils.h" #include "FLAC/assert.h" #include "FLAC/metadata.h" +#include "share/alloc.h" #include "share/grabbag.h" #include #include @@ -446,8 +447,8 @@ FLAC__bool do_shorthand_operation__add_replay_gain(char **filenames, unsigned nu } if( - 0 == (title_gains = (float*)malloc(sizeof(float) * num_files)) || - 0 == (title_peaks = (float*)malloc(sizeof(float) * num_files)) + 0 == (title_gains = (float*)safe_malloc_mul_2op_(sizeof(float), /*times*/num_files)) || + 0 == (title_peaks = (float*)safe_malloc_mul_2op_(sizeof(float), /*times*/num_files)) ) die("out of memory allocating space for title gains/peaks"); diff --git a/src/metaflac/options.c b/src/metaflac/options.c index 18b9d8e..56c4416 100644 --- a/src/metaflac/options.c +++ b/src/metaflac/options.c @@ -24,6 +24,7 @@ #include "usage.h" #include "utils.h" #include "FLAC/assert.h" +#include "share/alloc.h" #include "share/grabbag/replaygain.h" #include #include @@ -185,7 +186,7 @@ FLAC__bool parse_options(int argc, char *argv[], CommandLineOptions *options) if(options->num_files > 0) { unsigned i = 0; - if(0 == (options->filenames = (char**)malloc(sizeof(char*) * options->num_files))) + if(0 == (options->filenames = (char**)safe_malloc_mul_2op_(sizeof(char*), /*times*/options->num_files))) die("out of memory allocating space for file names list"); while(share__optind < argc) options->filenames[i++] = local_strdup(argv[share__optind++]); @@ -718,8 +719,10 @@ void append_new_operation(CommandLineOptions *options, Operation operation) } if(options->ops.capacity <= options->ops.num_operations) { unsigned original_capacity = options->ops.capacity; - options->ops.capacity *= 4; - if(0 == (options->ops.operations = (Operation*)realloc(options->ops.operations, sizeof(Operation) * options->ops.capacity))) + if(options->ops.capacity > SIZE_MAX / 2) /* overflow check */ + die("out of memory allocating space for option list"); + options->ops.capacity *= 2; + if(0 == (options->ops.operations = (Operation*)safe_realloc_mul_2op_(options->ops.operations, sizeof(Operation), /*times*/options->ops.capacity))) die("out of memory allocating space for option list"); memset(options->ops.operations + original_capacity, 0, sizeof(Operation) * (options->ops.capacity - original_capacity)); } @@ -737,8 +740,10 @@ void append_new_argument(CommandLineOptions *options, Argument argument) } if(options->args.capacity <= options->args.num_arguments) { unsigned original_capacity = options->args.capacity; - options->args.capacity *= 4; - if(0 == (options->args.arguments = (Argument*)realloc(options->args.arguments, sizeof(Argument) * options->args.capacity))) + if(options->args.capacity > SIZE_MAX / 2) /* overflow check */ + die("out of memory allocating space for option list"); + options->args.capacity *= 2; + if(0 == (options->args.arguments = (Argument*)safe_realloc_mul_2op_(options->args.arguments, sizeof(Argument), /*times*/options->args.capacity))) die("out of memory allocating space for option list"); memset(options->args.arguments + original_capacity, 0, sizeof(Argument) * (options->args.capacity - original_capacity)); } @@ -969,7 +974,7 @@ FLAC__bool parse_block_number(const char *in, Argument_BlockNumber *out) /* make space */ FLAC__ASSERT(out->num_entries > 0); - if(0 == (out->entries = (unsigned*)malloc(sizeof(unsigned) * out->num_entries))) + if(0 == (out->entries = (unsigned*)safe_malloc_mul_2op_(sizeof(unsigned), /*times*/out->num_entries))) die("out of memory allocating space for option list"); /* load 'em up */ @@ -1008,7 +1013,7 @@ FLAC__bool parse_block_type(const char *in, Argument_BlockType *out) /* make space */ FLAC__ASSERT(out->num_entries > 0); - if(0 == (out->entries = (Argument_BlockTypeEntry*)malloc(sizeof(Argument_BlockTypeEntry) * out->num_entries))) + if(0 == (out->entries = (Argument_BlockTypeEntry*)safe_malloc_mul_2op_(sizeof(Argument_BlockTypeEntry), /*times*/out->num_entries))) die("out of memory allocating space for option list"); /* load 'em up */ diff --git a/src/metaflac/utils.c b/src/metaflac/utils.c index b9bb5a0..cdd823c 100644 --- a/src/metaflac/utils.c +++ b/src/metaflac/utils.c @@ -22,6 +22,7 @@ #include "utils.h" #include "FLAC/assert.h" +#include "share/alloc.h" #include "share/utf8.h" #include #include @@ -57,7 +58,7 @@ char *local_strdup(const char *source) void local_strcat(char **dest, const char *source) { - unsigned ndest, nsource; + size_t ndest, nsource; FLAC__ASSERT(0 != dest); FLAC__ASSERT(0 != source); @@ -68,7 +69,7 @@ void local_strcat(char **dest, const char *source) if(nsource == 0) return; - *dest = (char*)realloc(*dest, ndest + nsource + 1); + *dest = (char*)safe_realloc_add_3op_(*dest, ndest, /*+*/nsource, /*+*/1); if(0 == *dest) die("out of memory growing string"); strcpy((*dest)+ndest, source); diff --git a/src/plugin_common/charset.c b/src/plugin_common/charset.c index df0bcfa..cc82669 100644 --- a/src/plugin_common/charset.c +++ b/src/plugin_common/charset.c @@ -83,6 +83,8 @@ char* FLAC_plugin__charset_convert_string (const char *string, char *from, char /* Due to a GLIBC bug, round outbuf_size up to a multiple of 4 */ /* + 1 for nul in case len == 1 */ outsize = ((length + 3) & ~3) + 1; + if(outsize < length) /* overflow check */ + return NULL; out = (char*)malloc(outsize); outleft = outsize - 1; outptr = out; @@ -95,6 +97,10 @@ retry: { case E2BIG: used = outptr - out; + if((outsize - 1) * 2 + 1 <= outsize) { /* overflow check */ + free(out); + return NULL; + } outsize = (outsize - 1) * 2 + 1; out = realloc(out, outsize); outptr = out + used; diff --git a/src/plugin_common/tags.c b/src/plugin_common/tags.c index 5f491d5..befc848 100644 --- a/src/plugin_common/tags.c +++ b/src/plugin_common/tags.c @@ -27,15 +27,16 @@ #include "tags.h" #include "FLAC/assert.h" #include "FLAC/metadata.h" +#include "share/alloc.h" #ifndef FLaC__INLINE #define FLaC__INLINE #endif -static FLaC__INLINE unsigned local__wide_strlen(const FLAC__uint16 *s) +static FLaC__INLINE size_t local__wide_strlen(const FLAC__uint16 *s) { - unsigned n = 0; + size_t n = 0; while(*s++) n++; return n; @@ -47,7 +48,7 @@ static FLaC__INLINE unsigned local__wide_strlen(const FLAC__uint16 *s) * and a more clear explanation at the end of this section: * http://www.cl.cam.ac.uk/~mgk25/unicode.html#utf-8 */ -static FLaC__INLINE unsigned local__utf8len(const FLAC__byte *utf8) +static FLaC__INLINE size_t local__utf8len(const FLAC__byte *utf8) { FLAC__ASSERT(0 != utf8); if ((utf8[0] & 0x80) == 0) { @@ -89,9 +90,9 @@ static FLaC__INLINE unsigned local__utf8len(const FLAC__byte *utf8) } -static FLaC__INLINE unsigned local__utf8_to_ucs2(const FLAC__byte *utf8, FLAC__uint16 *ucs2) +static FLaC__INLINE size_t local__utf8_to_ucs2(const FLAC__byte *utf8, FLAC__uint16 *ucs2) { - const unsigned len = local__utf8len(utf8); + const size_t len = local__utf8len(utf8); FLAC__ASSERT(0 != ucs2); @@ -110,7 +111,7 @@ static FLaC__INLINE unsigned local__utf8_to_ucs2(const FLAC__byte *utf8, FLAC__u static FLAC__uint16 *local__convert_utf8_to_ucs2(const char *src, unsigned length) { FLAC__uint16 *out; - unsigned chars = 0; + size_t chars = 0; FLAC__ASSERT(0 != src); @@ -127,7 +128,7 @@ static FLAC__uint16 *local__convert_utf8_to_ucs2(const char *src, unsigned lengt } /* allocate */ - out = (FLAC__uint16*)malloc(chars * sizeof(FLAC__uint16)); + out = (FLAC__uint16*)safe_malloc_mul_2op_(chars, /*times*/sizeof(FLAC__uint16)); if (0 == out) { FLAC__ASSERT(0); return 0; @@ -144,7 +145,7 @@ static FLAC__uint16 *local__convert_utf8_to_ucs2(const char *src, unsigned lengt return out; } -static FLaC__INLINE unsigned local__ucs2len(FLAC__uint16 ucs2) +static FLaC__INLINE size_t local__ucs2len(FLAC__uint16 ucs2) { if (ucs2 < 0x0080) return 1; @@ -154,7 +155,7 @@ static FLaC__INLINE unsigned local__ucs2len(FLAC__uint16 ucs2) return 3; } -static FLaC__INLINE unsigned local__ucs2_to_utf8(FLAC__uint16 ucs2, FLAC__byte *utf8) +static FLaC__INLINE size_t local__ucs2_to_utf8(FLAC__uint16 ucs2, FLAC__byte *utf8) { if (ucs2 < 0x080) { utf8[0] = (FLAC__byte)ucs2; @@ -176,19 +177,23 @@ static FLaC__INLINE unsigned local__ucs2_to_utf8(FLAC__uint16 ucs2, FLAC__byte * static char *local__convert_ucs2_to_utf8(const FLAC__uint16 *src, unsigned length) { char *out; - unsigned len = 0; + size_t len = 0, n; FLAC__ASSERT(0 != src); /* calculate length */ { unsigned i; - for (i = 0; i < length; i++) - len += local__ucs2len(src[i]); + for (i = 0; i < length; i++) { + n = local__ucs2len(src[i]); + if(len + n < len) /* overflow check */ + return 0; + len += n; + } } /* allocate */ - out = (char*)malloc(len * sizeof(char)); + out = (char*)safe_malloc_mul_2op_(len, /*times*/sizeof(char)); if (0 == out) return 0; @@ -311,7 +316,7 @@ FLAC__bool FLAC_plugin__tags_add_tag_utf8(FLAC__StreamMetadata *tags, const char const size_t value_len = strlen(value); const size_t separator_len = strlen(separator); FLAC__byte *new_entry; - if(0 == (new_entry = (FLAC__byte*)realloc(entry->entry, entry->length + value_len + separator_len + 1))) + if(0 == (new_entry = (FLAC__byte*)safe_realloc_add_4op_(entry->entry, entry->length, /*+*/value_len, /*+*/separator_len, /*+*/1))) return false; memcpy(new_entry+entry->length, separator, separator_len); entry->length += separator_len; diff --git a/src/plugin_winamp2/configure.c b/src/plugin_winamp2/configure.c index 51db3ac..400aef2 100644 --- a/src/plugin_winamp2/configure.c +++ b/src/plugin_winamp2/configure.c @@ -26,6 +26,7 @@ #include "configure.h" #include "tagz.h" #include "resource.h" +#include "share/alloc.h" static char buffer[256]; @@ -52,7 +53,7 @@ static wchar_t *convert_ansi_to_wide_(const char *src) len = strlen(src) + 1; /* copy */ - dest = malloc(len*sizeof(wchar_t)); + dest = safe_malloc_mul_2op_(len, /*times*/sizeof(wchar_t)); if (dest) mbstowcs(dest, src, len); return dest; } diff --git a/src/plugin_winamp2/in_flac.c b/src/plugin_winamp2/in_flac.c index 1587a09..b79fb8d 100644 --- a/src/plugin_winamp2/in_flac.c +++ b/src/plugin_winamp2/in_flac.c @@ -24,6 +24,7 @@ #include /* for INT_MAX */ #include +#include "share/alloc.h" #include "winamp2/in2.h" #include "configure.h" #include "infobox.h" @@ -279,7 +280,7 @@ static T_CHAR *get_tag(const T_CHAR *tag, void *param) if (!tag) return 0; /* Vorbis comment names must be ASCII, so convert 'tag' first */ - tagname = malloc(wcslen(tag)+1); + tagname = safe_malloc_add_2op_(wcslen(tag), /*+*/1); for(p=tagname;*tag;) { if(*tag > 0x7d) { free(tagname); diff --git a/src/plugin_winamp2/infobox.c b/src/plugin_winamp2/infobox.c index c4b8034..9418eeb 100644 --- a/src/plugin_winamp2/infobox.c +++ b/src/plugin_winamp2/infobox.c @@ -23,6 +23,7 @@ #include #include #include "FLAC/all.h" +#include "share/alloc.h" #include "plugin_common/all.h" #include "infobox.h" #include "configure.h" @@ -74,7 +75,7 @@ static void LoadGenres() hFile = CreateFile(buffer, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (hFile == INVALID_HANDLE_VALUE) return; genresSize = GetFileSize(hFile, 0); - if (genresSize && (genres = (char*)malloc(genresSize+2))) + if (genresSize && (genres = (char*)safe_malloc_add_2op_(genresSize, /*+*/2))) { if (!ReadFile(hFile, genres, genresSize, &spam, NULL) || spam!=genresSize) { @@ -187,7 +188,7 @@ static wchar_t *AnsiToWide(const char *src) len = strlen(src) + 1; /* copy */ - dest = (wchar_t*)malloc(len*sizeof(wchar_t)); + dest = (wchar_t*)safe_malloc_mul_2op_(len, /*times*/sizeof(wchar_t)); if (dest) mbstowcs(dest, src, len); return dest; } diff --git a/src/plugin_xmms/plugin.c b/src/plugin_xmms/plugin.c index e46d29a..24a79b7 100644 --- a/src/plugin_xmms/plugin.c +++ b/src/plugin_xmms/plugin.c @@ -400,8 +400,13 @@ void FLAC_XMMS__get_song_info(char *filename, char **title, int *length_in_msec) if(title) { if (!is_http_source(filename)) { static const char *errtitle = "Invalid FLAC File: "; - *title = g_malloc(strlen(errtitle) + 1 + strlen(filename) + 1 + 1); - sprintf(*title, "%s\"%s\"", errtitle, filename); + if(strlen(errtitle) + 1 + strlen(filename) + 1 + 1 < strlen(filename)) { /* overflow check */ + *title = NULL; + } + else { + *title = g_malloc(strlen(errtitle) + 1 + strlen(filename) + 1 + 1); + sprintf(*title, "%s\"%s\"", errtitle, filename); + } } else { *title = NULL; } diff --git a/src/share/grabbag/picture.c b/src/share/grabbag/picture.c index 09aff7d..0e37273 100644 --- a/src/share/grabbag/picture.c +++ b/src/share/grabbag/picture.c @@ -20,6 +20,7 @@ # include #endif +#include "share/alloc.h" #include "share/grabbag.h" #include "FLAC/assert.h" #include @@ -29,7 +30,7 @@ /* slightly different that strndup(): this always copies 'size' bytes starting from s into a NUL-terminated string. */ static char *local__strndup_(const char *s, size_t size) { - char *x = (char*)malloc(size+1); + char *x = (char*)safe_malloc_add_2op_(size, /*+*/1); if(x) { memcpy(x, s, size); x[size] = '\0'; @@ -357,7 +358,7 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con if(size < 0) *error_message = error_messages[5]; else { - FLAC__byte *buffer = (FLAC__byte*)malloc(size); + FLAC__byte *buffer = (FLAC__byte*)safe_malloc_(size); if(0 == buffer) *error_message = error_messages[0]; else { diff --git a/src/share/utf8/charset.c b/src/share/utf8/charset.c index 3d13064..dcef6fc 100644 --- a/src/share/utf8/charset.c +++ b/src/share/utf8/charset.c @@ -35,6 +35,7 @@ #include +#include "share/alloc.h" #include "charset.h" #include "charmaps.h" @@ -492,7 +493,7 @@ int charset_convert(const char *fromcode, const char *tocode, if (!charset1 || !charset2 ) return -1; - tobuf = (char *)malloc(fromlen * charset2->max + 1); + tobuf = (char *)safe_malloc_mul2add_(fromlen, /*times*/charset2->max, /*+*/1); if (!tobuf) return -2; diff --git a/src/share/utf8/iconvert.c b/src/share/utf8/iconvert.c index 37f16a8..f0311ee 100644 --- a/src/share/utf8/iconvert.c +++ b/src/share/utf8/iconvert.c @@ -29,6 +29,7 @@ #include #include "iconvert.h" +#include "share/alloc.h" /* * Convert data from one encoding to another. Return: @@ -81,7 +82,7 @@ int iconvert(const char *fromcode, const char *tocode, * This is deliberately not a config option as people often * change their iconv library without rebuilding applications. */ - tocode1 = (char *)malloc(strlen(tocode) + 11); + tocode1 = (char *)safe_malloc_add_2op_(strlen(tocode), /*+*/11); if (!tocode1) goto fail; @@ -119,6 +120,8 @@ int iconvert(const char *fromcode, const char *tocode, break; if (obl < 6) { /* Enlarge the buffer */ + if(utflen*2 < utflen) /* overflow check */ + goto fail; utflen *= 2; newbuf = (char *)realloc(utfbuf, utflen); if (!newbuf) @@ -145,7 +148,7 @@ int iconvert(const char *fromcode, const char *tocode, iconv_close(cd1); return ret; } - newbuf = (char *)realloc(utfbuf, (ob - utfbuf) + 1); + newbuf = (char *)safe_realloc_add_2op_(utfbuf, (ob - utfbuf), /*+*/1); if (!newbuf) goto fail; ob = (ob - utfbuf) + newbuf; @@ -196,7 +199,7 @@ int iconvert(const char *fromcode, const char *tocode, outlen += ob - tbuf; /* Convert from UTF-8 for real */ - outbuf = (char *)malloc(outlen + 1); + outbuf = (char *)safe_malloc_add_2op_(outlen, /*+*/1); if (!outbuf) goto fail; ib = utfbuf; diff --git a/src/share/utf8/utf8.c b/src/share/utf8/utf8.c index f2bf570..303fe23 100644 --- a/src/share/utf8/utf8.c +++ b/src/share/utf8/utf8.c @@ -2,6 +2,8 @@ * Copyright (C) 2001 Peter Harris * Copyright (C) 2001 Edmund Grimley Evans * + * Buffer overflow checking added: Josh Coalson, 9/9/2007 + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -28,6 +30,7 @@ #include #include +#include "share/alloc.h" #include "utf8.h" #include "charset.h" @@ -43,7 +46,8 @@ static unsigned char *make_utf8_string(const wchar_t *unicode) { - int size = 0, index = 0, out_index = 0; + size_t size = 0, n; + int index = 0, out_index = 0; unsigned char *out; unsigned short c; @@ -51,16 +55,19 @@ static unsigned char *make_utf8_string(const wchar_t *unicode) c = unicode[index++]; while(c) { if(c < 0x0080) { - size += 1; + n = 1; } else if(c < 0x0800) { - size += 2; + n = 2; } else { - size += 3; + n = 3; } + if(size+n < size) /* overflow check */ + return NULL; + size += n; c = unicode[index++]; - } + } - out = malloc(size + 1); + out = safe_malloc_add_2op_(size, /*+*/1); if (out == NULL) return NULL; index = 0; @@ -87,7 +94,8 @@ static unsigned char *make_utf8_string(const wchar_t *unicode) static wchar_t *make_unicode_string(const unsigned char *utf8) { - int size = 0, index = 0, out_index = 0; + size_t size = 0; + int index = 0, out_index = 0; wchar_t *out; unsigned char c; @@ -101,11 +109,15 @@ static wchar_t *make_unicode_string(const unsigned char *utf8) } else { index += 1; } - size += 1; + if(size + 1 == 0) /* overflow check */ + return NULL; + size++; c = utf8[index++]; - } + } - out = malloc((size + 1) * sizeof(wchar_t)); + if(size + 1 == 0) /* overflow check */ + return NULL; + out = safe_malloc_mul_2op_(size+1, /*times*/sizeof(wchar_t)); if (out == NULL) return NULL; index = 0; @@ -147,7 +159,10 @@ int utf8_encode(const char *from, char **to) return -1; } - unicode = calloc(wchars + 1, sizeof(unsigned short)); + if(wchars < 0) /* underflow check */ + return -1; + + unicode = safe_calloc_((size_t)wchars + 1, sizeof(unsigned short)); if(unicode == NULL) { fprintf(stderr, "Out of memory processing string to UTF8\n"); @@ -190,6 +205,9 @@ int utf8_decode(const char *from, char **to) chars = WideCharToMultiByte(GetConsoleCP(), WC_COMPOSITECHECK, unicode, -1, NULL, 0, NULL, NULL); + if(chars < 0) /* underflow check */ + return -1; + if(chars == 0) { fprintf(stderr, "Unicode translation error %d\n", GetLastError()); @@ -197,7 +215,7 @@ int utf8_decode(const char *from, char **to) return -1; } - *to = calloc(chars + 1, sizeof(unsigned char)); + *to = safe_calloc_((size_t)chars + 1, sizeof(unsigned char)); if(*to == NULL) { fprintf(stderr, "Out of memory processing string to local charset\n"); @@ -277,7 +295,7 @@ static int convert_string(const char *fromcode, const char *tocode, if (ret != -1) return ret; - s = malloc(fromlen + 1); + s = safe_malloc_add_2op_(fromlen, /*+*/1); if (!s) return -1; strcpy(s, from);