mirror of https://github.com/mpv-player/mpv
demux_mkv: better check for some EBML parsing
Apparently, originally this code was meant to be able to read past the buffer somewhat, which is why the buffer allocation was padded by 8 byte. This is unclean and confuses valgrind. This probably could have crashed with certain invalid files too. Also revert the change added with 10a2f69; it should be not needed anymore.
This commit is contained in:
parent
866a5a12b9
commit
f3a978cd17
34
demux/ebml.c
34
demux/ebml.c
|
@ -279,8 +279,11 @@ static uint32_t ebml_parse_id(uint8_t *data, int *length)
|
|||
return id;
|
||||
}
|
||||
|
||||
static uint64_t parse_vlen(uint8_t *data, int *length, bool is_length)
|
||||
static uint64_t ebml_parse_length(uint8_t *data, size_t data_len, int *length)
|
||||
{
|
||||
uint8_t *end = data + data_len;
|
||||
if (data == end)
|
||||
return -1;
|
||||
uint64_t r = *data++;
|
||||
int len = 1;
|
||||
int len_mask;
|
||||
|
@ -297,11 +300,13 @@ static uint64_t parse_vlen(uint8_t *data, int *length, bool is_length)
|
|||
if (r == len_mask - 1)
|
||||
num_allones++;
|
||||
for (int i = 1; i < len; i++) {
|
||||
if (data == end)
|
||||
return -1;
|
||||
if (*data == 255)
|
||||
num_allones++;
|
||||
r = (r << 8) | *data++;
|
||||
}
|
||||
if (is_length && num_allones == len) {
|
||||
if (num_allones == len) {
|
||||
// According to Matroska specs this means "unknown length"
|
||||
// Could be supported if there are any actual files using it
|
||||
*length = -1;
|
||||
|
@ -311,11 +316,6 @@ static uint64_t parse_vlen(uint8_t *data, int *length, bool is_length)
|
|||
return r;
|
||||
}
|
||||
|
||||
static uint64_t ebml_parse_length(uint8_t *data, int *length)
|
||||
{
|
||||
return parse_vlen(data, length, true);
|
||||
}
|
||||
|
||||
static uint64_t ebml_parse_uint(uint8_t *data, int length)
|
||||
{
|
||||
assert(length >= 1 && length <= 8);
|
||||
|
@ -371,7 +371,7 @@ static void ebml_parse_element(struct ebml_parse_ctx *ctx, void *target,
|
|||
goto other_error;
|
||||
}
|
||||
p += len;
|
||||
uint64_t length = ebml_parse_length(p, &len);
|
||||
uint64_t length = ebml_parse_length(p, end - p, &len);
|
||||
if (len > end - p)
|
||||
goto past_end_error;
|
||||
if (len < 0) {
|
||||
|
@ -459,10 +459,16 @@ static void ebml_parse_element(struct ebml_parse_ctx *ctx, void *target,
|
|||
while (data < end) {
|
||||
int len;
|
||||
uint32_t id = ebml_parse_id(data, &len);
|
||||
assert(len >= 0 && len <= end - data);
|
||||
if (len < 0 || len > end - data) {
|
||||
MP_DBG(ctx, "Error parsing subelement\n");
|
||||
break;
|
||||
}
|
||||
data += len;
|
||||
uint64_t length = ebml_parse_length(data, &len);
|
||||
assert(len >= 0 && len <= end - data);
|
||||
uint64_t length = ebml_parse_length(data, end - data, &len);
|
||||
if (len < 0 || len > end - data) {
|
||||
MP_DBG(ctx, "Error parsing subelement length\n");
|
||||
break;
|
||||
}
|
||||
data += len;
|
||||
if (length > end - data) {
|
||||
// Try to parse what is possible from inside this partial element
|
||||
|
@ -614,12 +620,10 @@ int ebml_read_element(struct stream *s, struct ebml_parse_ctx *ctx,
|
|||
MP_MSG(ctx, msglevel, "Refusing to read element over 100 MB in size\n");
|
||||
return -1;
|
||||
}
|
||||
ctx->talloc_ctx = talloc_size(NULL, length + 8);
|
||||
ctx->talloc_ctx = talloc_size(NULL, length);
|
||||
int read_len = stream_read(s, ctx->talloc_ctx, length);
|
||||
if (read_len < length) {
|
||||
if (read_len < length)
|
||||
MP_MSG(ctx, msglevel, "Unexpected end of file - partial or corrupt file?\n");
|
||||
memset((char *)ctx->talloc_ctx + read_len, 0, length - read_len);
|
||||
}
|
||||
ebml_parse_element(ctx, target, ctx->talloc_ctx, read_len, desc, 0);
|
||||
if (ctx->has_errors)
|
||||
MP_MSG(ctx, msglevel, "Error parsing element %s\n", desc->name);
|
||||
|
|
Loading…
Reference in New Issue