From 71f504a43d66c48ef0494be5e48c626e9a6eba06 Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 27 Nov 2014 21:54:37 +0100 Subject: demux_mkv: fix a possible out of bounds access The if branch has a weak check to test whether the codec_id is the short ID, and handles the long IDs in the else branch. The long IDs are all longer than 12 bytes long, so hardcoding the string offset to get the trailing part of the name makes sense. But the if condition checks for another thing, which could get the else branch run even if the codec_id is short. Fix the bogus control flow and check if the codec_id is long enough. One of these checks could be considered redundant, but include them both for defensive coding. --- demux/demux_mkv.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index 31410770c6..d77c52a581 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -1414,20 +1414,22 @@ static int demux_mkv_open_audio(demuxer_t *demuxer, mkv_track_t *track) sh_a->bitrate = 16000 * 8; sh_a->block_align = 1024; - if (!strcmp(track->codec_id, MKV_A_AAC) && track->private_data) { - if (!extradata_len) { + if (!strcmp(track->codec_id, MKV_A_AAC)) { + if (track->private_data && !extradata_len) { extradata = track->private_data; extradata_len = track->private_size; } } else { /* Recreate the 'private data' */ - /* which faad2 uses in its initialization */ srate_idx = aac_get_sample_rate_index(track->a_sfreq); - if (!strncmp(&track->codec_id[12], "MAIN", 4)) + const char *tail = ""; + if (strlen(track->codec_id) >= 12) + tail = &track->codec_id[12]; + if (!strncmp(tail, "MAIN", 4)) profile = 0; - else if (!strncmp(&track->codec_id[12], "LC", 2)) + else if (!strncmp(tail, "LC", 2)) profile = 1; - else if (!strncmp(&track->codec_id[12], "SSR", 3)) + else if (!strncmp(tail, "SSR", 3)) profile = 2; else profile = 3; -- cgit v1.2.3