demux_mkv: fix EBML parsing checks

Reading IDs must be checked too. This was basically forgotten in commit
f3a978cd. Also set the *length parameter for ebml_parse_length() in some
error cases, which _really_ should happen.

Fixes #1461.
This commit is contained in:
wm4 2015-01-12 14:31:16 +01:00 committed by Diogo Franco (Kovensky)
parent 1a3a52ea25
commit 42f82c5ccf
1 changed files with 15 additions and 16 deletions

View File

@ -262,25 +262,28 @@ int ebml_resync_cluster(struct mp_log *log, stream_t *s)
struct generic; struct generic;
#define generic_struct struct generic #define generic_struct struct generic
static uint32_t ebml_parse_id(uint8_t *data, int *length) static uint32_t ebml_parse_id(uint8_t *data, size_t data_len, int *length)
{ {
*length = -1;
uint8_t *end = data + data_len;
if (data == end)
return EBML_ID_INVALID;
int len = 1; int len = 1;
uint32_t id = *data++; uint32_t id = *data++;
for (int len_mask = 0x80; !(id & len_mask); len_mask >>= 1) { for (int len_mask = 0x80; !(id & len_mask); len_mask >>= 1) {
len++; len++;
if (len > 4) { if (len > 4)
*length = -1;
return EBML_ID_INVALID; return EBML_ID_INVALID;
}
} }
*length = len; *length = len;
while (--len) while (--len && data < end)
id = (id << 8) | *data++; id = (id << 8) | *data++;
return id; return id;
} }
static uint64_t ebml_parse_length(uint8_t *data, size_t data_len, int *length) static uint64_t ebml_parse_length(uint8_t *data, size_t data_len, int *length)
{ {
*length = -1;
uint8_t *end = data + data_len; uint8_t *end = data + data_len;
if (data == end) if (data == end)
return -1; return -1;
@ -289,10 +292,8 @@ static uint64_t ebml_parse_length(uint8_t *data, size_t data_len, int *length)
int len_mask; int len_mask;
for (len_mask = 0x80; !(r & len_mask); len_mask >>= 1) { for (len_mask = 0x80; !(r & len_mask); len_mask >>= 1) {
len++; len++;
if (len > 8) { if (len > 8)
*length = -1;
return -1; return -1;
}
} }
r &= len_mask - 1; r &= len_mask - 1;
@ -306,12 +307,10 @@ static uint64_t ebml_parse_length(uint8_t *data, size_t data_len, int *length)
num_allones++; num_allones++;
r = (r << 8) | *data++; r = (r << 8) | *data++;
} }
if (num_allones == len) { // According to Matroska specs this means "unknown length"
// According to Matroska specs this means "unknown length" // Could be supported if there are any actual files using it
// Could be supported if there are any actual files using it if (num_allones == len)
*length = -1;
return -1; return -1;
}
*length = len; *length = len;
return r; return r;
} }
@ -363,7 +362,7 @@ static void ebml_parse_element(struct ebml_parse_ctx *ctx, void *target,
while (p < end) { while (p < end) {
uint8_t *startp = p; uint8_t *startp = p;
int len; int len;
uint32_t id = ebml_parse_id(p, &len); uint32_t id = ebml_parse_id(p, end - p, &len);
if (len > end - p) if (len > end - p)
goto past_end_error; goto past_end_error;
if (len < 0) { if (len < 0) {
@ -458,7 +457,7 @@ static void ebml_parse_element(struct ebml_parse_ctx *ctx, void *target,
while (data < end) { while (data < end) {
int len; int len;
uint32_t id = ebml_parse_id(data, &len); uint32_t id = ebml_parse_id(data, end - data, &len);
if (len < 0 || len > end - data) { if (len < 0 || len > end - data) {
MP_DBG(ctx, "Error parsing subelement\n"); MP_DBG(ctx, "Error parsing subelement\n");
break; break;
@ -588,7 +587,7 @@ static void ebml_parse_element(struct ebml_parse_ctx *ctx, void *target,
case EBML_TYPE_EBML_ID:; case EBML_TYPE_EBML_ID:;
uint32_t *idptr; uint32_t *idptr;
GETPTR(idptr, uint32_t); GETPTR(idptr, uint32_t);
*idptr = ebml_parse_id(data, &len); *idptr = ebml_parse_id(data, end - data, &len);
if (len != length) { if (len != length) {
MP_DBG(ctx, "ebml_id broken value\n"); MP_DBG(ctx, "ebml_id broken value\n");
goto error; goto error;