extra checking on memory allocation sizes to prevent a class of overflow attacks

This commit is contained in:
Josh Coalson
2007-09-11 04:49:56 +00:00
parent 0221d87c89
commit 0f008d2e9e
26 changed files with 234 additions and 115 deletions

View File

@@ -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;
}