Skip to content

Commit

Permalink
reject vorbis comments with negative loop parameters (bug #4905)
Browse files Browse the repository at this point in the history 
Also simplify music->loop decision,  change FLAC_Music->loop_start,
loop_len, loop_end, pcm_pos and full_length members to signed type.
  • Loading branch information
@sezero
sezero committed Dec 19, 2019
1 parent a779b03 commit e819489
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 63 deletions.
64 changes: 32 additions & 32 deletions src/codecs/music_flac.c
View file
Expand Up @@ -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;


Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
32 changes: 17 additions & 15 deletions src/codecs/music_ogg.c
View file
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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 */
Expand All @@ -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;
Expand Down Expand Up @@ -311,20 +311,26 @@ 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);
}

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

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

Expand Down
36 changes: 20 additions & 16 deletions src/codecs/music_opus.c
View file
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

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

Expand Down

0 comments on commit e819489

Please sign in to comment.