From e819489459d5e9942851b9547854ba988fe50291 Mon Sep 17 00:00:00 2001 From: Ozkan Sezer Date: Thu, 19 Dec 2019 18:55:02 +0300 Subject: [PATCH] reject vorbis comments with negative loop parameters (bug #4905) Also simplify music->loop decision, change FLAC_Music->loop_start, loop_len, loop_end, pcm_pos and full_length members to signed type. --- src/codecs/music_flac.c | 64 ++++++++++++++++++++--------------------- src/codecs/music_ogg.c | 32 +++++++++++---------- src/codecs/music_opus.c | 36 ++++++++++++----------- 3 files changed, 69 insertions(+), 63 deletions(-) diff --git a/src/codecs/music_flac.c b/src/codecs/music_flac.c index 7d1a717f..e26f6fa0 100644 --- a/src/codecs/music_flac.c +++ b/src/codecs/music_flac.c @@ -155,12 +155,12 @@ typedef struct { int freesrc; SDL_AudioStream *stream; int loop; - FLAC__uint64 pcm_pos; - FLAC__uint64 full_length; + FLAC__int64 pcm_pos; + FLAC__int64 full_length; SDL_bool loop_flag; - FLAC__uint64 loop_start; - FLAC__uint64 loop_end; - FLAC__uint64 loop_len; + FLAC__int64 loop_start; + FLAC__int64 loop_end; + FLAC__int64 loop_len; } FLAC_Music; @@ -351,7 +351,7 @@ static FLAC__StreamDecoderWriteStatus flac_write_music_cb( } } amount = (int)(frame->header.blocksize * channels * sizeof(*data)); - music->pcm_pos += (FLAC__uint64) frame->header.blocksize; + music->pcm_pos += (FLAC__int64) frame->header.blocksize; if ((music->loop == 1) && (music->play_count != 1) && (music->pcm_pos >= music->loop_end)) { amount -= (music->pcm_pos - music->loop_end) * channels * sizeof(*data); @@ -366,15 +366,15 @@ static FLAC__StreamDecoderWriteStatus flac_write_music_cb( /* Parse time string of the form HH:MM:SS.mmm and return equivalent sample * position */ -static FLAC__uint64 parse_time(char *time, unsigned samplerate_hz) +static FLAC__int64 parse_time(char *time, unsigned samplerate_hz) { char *num_start, *p; - FLAC__uint64 result = 0; - char c; + FLAC__int64 result = 0; + char c; int val; /* Time is directly expressed as a sample position */ if (SDL_strchr(time, ':') == NULL) { - return SDL_strtoull(time, NULL, 10); + return SDL_strtoll(time, NULL, 10); } result = 0; @@ -383,18 +383,22 @@ static FLAC__uint64 parse_time(char *time, unsigned samplerate_hz) for (p = time; *p != '\0'; ++p) { if (*p == '.' || *p == ':') { c = *p; *p = '\0'; - result = result * 60 + SDL_atoi(num_start); + if ((val = SDL_atoi(num_start)) < 0) + return -1; + result = result * 60 + val; num_start = p + 1; *p = c; } if (*p == '.') { - return result * samplerate_hz - + (FLAC__uint64) (SDL_atof(p) * samplerate_hz); + double val_f = SDL_atof(p); + if (val_f < 0) return -1; + return result * samplerate_hz + (FLAC__int64) (val_f * samplerate_hz); } } - return (result * 60 + SDL_atoi(num_start)) * samplerate_hz; + if ((val = SDL_atoi(num_start)) < 0) return -1; + return (result * 60 + val) * samplerate_hz; } static void flac_metadata_music_cb( @@ -449,21 +453,26 @@ static void flac_metadata_music_cb( /* Want to match LOOP-START, LOOP_START, etc. Remove - or _ from * string if it is present at position 4. */ - if ((argument[4] == '_') || (argument[4] == '-')) { - SDL_memmove(argument + 4, argument + 5, - SDL_strlen(argument) - 4); + SDL_memmove(argument + 4, argument + 5, SDL_strlen(argument) - 4); } if (SDL_strcasecmp(argument, "LOOPSTART") == 0) music->loop_start = parse_time(value, rate); else if (SDL_strcasecmp(argument, "LOOPLENGTH") == 0) { - music->loop_len = SDL_strtoull(value, NULL, 10); + music->loop_len = SDL_strtoll(value, NULL, 10); is_loop_length = SDL_TRUE; } else if (SDL_strcasecmp(argument, "LOOPEND") == 0) { music->loop_end = parse_time(value, rate); is_loop_length = SDL_FALSE; } + if (music->loop_start < 0 || music->loop_len < 0 || music->loop_end < 0) { + music->loop_start = 0; + music->loop_len = 0; + music->loop_end = 0; + SDL_free(param); + break; /* ignore tag. */ + } SDL_free(param); } @@ -509,7 +518,7 @@ static void *FLAC_CreateFromRW(SDL_RWops *src, int freesrc) FLAC_Music *music; int init_stage = 0; int was_error = 1; - FLAC__uint64 full_length; + FLAC__int64 full_length; music = (FLAC_Music *)SDL_calloc(1, sizeof(*music)); if (!music) { @@ -519,11 +528,6 @@ static void *FLAC_CreateFromRW(SDL_RWops *src, int freesrc) music->src = src; music->volume = MIX_MAX_VOLUME; music->loop = -1; - music->loop_start = -1; - music->loop_end = 0; - music->loop_len = 0; - music->loop_flag = SDL_FALSE; - music->pcm_pos = 0; music->flac_decoder = flac.FLAC__stream_decoder_new(); if (music->flac_decoder) { @@ -568,13 +572,9 @@ static void *FLAC_CreateFromRW(SDL_RWops *src, int freesrc) /* loop_start, loop_end and loop_len get set by metadata callback if tags * are present in metadata. */ - full_length = flac.FLAC__stream_decoder_get_total_samples(music->flac_decoder); - if (((music->loop_start >= 0) || (music->loop_end > 0)) && - ((music->loop_start < music->loop_end) || (music->loop_end > 0)) && - (music->loop_start < full_length) && - (music->loop_end <= full_length)) { - if (music->loop_start < 0) music->loop_start = 0; - if (music->loop_end == 0) music->loop_end = full_length; + full_length = (FLAC__int64) flac.FLAC__stream_decoder_get_total_samples(music->flac_decoder); + if ((music->loop_end > 0) && (music->loop_end <= full_length) && + (music->loop_start < music->loop_end)) { music->loop = 1; } @@ -668,7 +668,7 @@ static int FLAC_Seek(void *context, double position) FLAC__uint64 seek_sample = (FLAC__uint64) (music->sample_rate * position); SDL_AudioStreamClear(music->stream); - music->pcm_pos = seek_sample; + music->pcm_pos = (FLAC__int64) seek_sample; if (!flac.FLAC__stream_decoder_seek_absolute(music->flac_decoder, seek_sample)) { if (flac.FLAC__stream_decoder_get_state(music->flac_decoder) == FLAC__STREAM_DECODER_SEEK_ERROR) { flac.FLAC__stream_decoder_flush(music->flac_decoder); diff --git a/src/codecs/music_ogg.c b/src/codecs/music_ogg.c index 09bd6470..384b17a8 100644 --- a/src/codecs/music_ogg.c +++ b/src/codecs/music_ogg.c @@ -230,11 +230,11 @@ static ogg_int64_t parse_time(char *time, long samplerate_hz) { char *num_start, *p; ogg_int64_t result = 0; - char c; + char c; int val; /* Time is directly expressed as a sample position */ if (SDL_strchr(time, ':') == NULL) { - return (ogg_int64_t)SDL_strtoull(time, NULL, 10); + return SDL_strtoll(time, NULL, 10); } result = 0; @@ -243,7 +243,9 @@ static ogg_int64_t parse_time(char *time, long samplerate_hz) for (p = time; *p != '\0'; ++p) { if (*p == '.' || *p == ':') { c = *p; *p = '\0'; - result = result * 60 + SDL_atoi(num_start); + if ((val = SDL_atoi(num_start)) < 0) + return -1; + result = result * 60 + val; num_start = p + 1; *p = c; } @@ -254,7 +256,8 @@ static ogg_int64_t parse_time(char *time, long samplerate_hz) } } - return (result * 60 + SDL_atoi(num_start)) * samplerate_hz; + if ((val = SDL_atoi(num_start)) < 0) return -1; + return (result * 60 + val) * samplerate_hz; } /* Load an OGG stream from an SDL_RWops object */ @@ -277,9 +280,6 @@ static void *OGG_CreateFromRW(SDL_RWops *src, int freesrc) music->volume = MIX_MAX_VOLUME; music->section = -1; music->loop = -1; - music->loop_start = -1; - music->loop_end = 0; - music->loop_len = 0; SDL_zero(callbacks); callbacks.read_func = sdl_read_func; @@ -311,7 +311,6 @@ static void *OGG_CreateFromRW(SDL_RWops *src, int freesrc) /* Want to match LOOP-START, LOOP_START, etc. Remove - or _ from * string if it is present at position 4. */ - if ((argument[4] == '_') || (argument[4] == '-')) { SDL_memmove(argument + 4, argument + 5, SDL_strlen(argument) - 4); } @@ -319,12 +318,19 @@ static void *OGG_CreateFromRW(SDL_RWops *src, int freesrc) if (SDL_strcasecmp(argument, "LOOPSTART") == 0) music->loop_start = parse_time(value, rate); else if (SDL_strcasecmp(argument, "LOOPLENGTH") == 0) { - music->loop_len = (ogg_int64_t)SDL_strtoull(value, NULL, 10); + music->loop_len = SDL_strtoll(value, NULL, 10); is_loop_length = SDL_TRUE; } else if (SDL_strcasecmp(argument, "LOOPEND") == 0) { music->loop_end = parse_time(value, rate); is_loop_length = SDL_FALSE; } + if (music->loop_start < 0 || music->loop_len < 0 || music->loop_end < 0) { + music->loop_start = 0; + music->loop_len = 0; + music->loop_end = 0; + SDL_free(param); + break; /* ignore tag. */ + } SDL_free(param); } @@ -335,12 +341,8 @@ static void *OGG_CreateFromRW(SDL_RWops *src, int freesrc) } full_length = vorbis.ov_pcm_total(&music->vf, -1); - if (((music->loop_start >= 0) || (music->loop_end > 0)) && - ((music->loop_start < music->loop_end) || (music->loop_end > 0)) && - (music->loop_start < full_length) && - (music->loop_end <= full_length)) { - if (music->loop_start < 0) music->loop_start = 0; - if (music->loop_end == 0) music->loop_end = full_length; + if ((music->loop_end > 0) && (music->loop_end <= full_length) && + (music->loop_start < music->loop_end)) { music->loop = 1; } diff --git a/src/codecs/music_opus.c b/src/codecs/music_opus.c index 9dff3c53..ee3f2fb3 100644 --- a/src/codecs/music_opus.c +++ b/src/codecs/music_opus.c @@ -214,11 +214,11 @@ static ogg_int64_t parse_time(char *time) const ogg_int64_t samplerate_hz = 48000; char *num_start, *p; ogg_int64_t result = 0; - char c; + char c; int val; /* Time is directly expressed as a sample position */ if (SDL_strchr(time, ':') == NULL) { - return (ogg_int64_t)SDL_strtoull(time, NULL, 10); + return SDL_strtoll(time, NULL, 10); } result = 0; @@ -227,18 +227,22 @@ static ogg_int64_t parse_time(char *time) for (p = time; *p != '\0'; ++p) { if (*p == '.' || *p == ':') { c = *p; *p = '\0'; - result = result * 60 + SDL_atoi(num_start); + if ((val = SDL_atoi(num_start)) < 0) + return -1; + result = result * 60 + val; num_start = p + 1; *p = c; } if (*p == '.') { - return result * samplerate_hz - + (ogg_int64_t) (SDL_atof(p) * samplerate_hz); + double val_f = SDL_atof(p); + if (val_f < 0) return -1; + return result * samplerate_hz + (ogg_int64_t) (val_f * samplerate_hz); } } - return (result * 60 + SDL_atoi(num_start)) * samplerate_hz; + if ((val = SDL_atoi(num_start)) < 0) return -1; + return (result * 60 + val) * samplerate_hz; } static SDL_bool is_loop_tag(const char *tag) @@ -267,9 +271,6 @@ static void *OPUS_CreateFromRW(SDL_RWops *src, int freesrc) music->volume = MIX_MAX_VOLUME; music->section = -1; music->loop = -1; - music->loop_start = -1; - music->loop_end = 0; - music->loop_len = 0; SDL_zero(callbacks); callbacks.read = sdl_read_func; @@ -315,12 +316,19 @@ static void *OPUS_CreateFromRW(SDL_RWops *src, int freesrc) if (SDL_strcasecmp(argument, "LOOPSTART") == 0) music->loop_start = parse_time(value); else if (SDL_strcasecmp(argument, "LOOPLENGTH") == 0) { - music->loop_len = (ogg_int64_t)SDL_strtoull(value, NULL, 10); + music->loop_len = SDL_strtoll(value, NULL, 10); is_loop_length = SDL_TRUE; } else if (SDL_strcasecmp(argument, "LOOPEND") == 0) { music->loop_end = parse_time(value); is_loop_length = SDL_FALSE; } + if (music->loop_start < 0 || music->loop_len < 0 || music->loop_end < 0) { + music->loop_start = 0; + music->loop_len = 0; + music->loop_end = 0; + SDL_free(param); + break; /* ignore tag. */ + } SDL_free(param); } @@ -331,12 +339,8 @@ static void *OPUS_CreateFromRW(SDL_RWops *src, int freesrc) } full_length = opus.op_pcm_total(music->of, -1); - if (((music->loop_start >= 0) || (music->loop_end > 0)) && - ((music->loop_start < music->loop_end) || (music->loop_end > 0)) && - (music->loop_start < full_length) && - (music->loop_end <= full_length)) { - if (music->loop_start < 0) music->loop_start = 0; - if (music->loop_end == 0) music->loop_end = full_length; + if ((music->loop_end > 0) && (music->loop_end <= full_length) && + (music->loop_start < music->loop_end)) { music->loop = 1; }