From 1177d16193c6308c4cdffb9f0ea69ce731c8b1c1 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Fri, 20 Aug 2021 14:23:04 +0200 Subject: [PATCH] mp4: Improve parse_tag(). * Merge tag_add_field() and read_string() into parse_tag() since they are simple enough and have only one caller. * Avoid memory leaks in the error case. * Let the function return an error code (rather than -1) in all cases, and check the return value in the callers. * Add a sanity check for the subsize. * Avoid creating two copies of the tag value. * Rename the variable for the tag value. --- mp4.c | 85 ++++++++++++++++++++++++----------------------------------- 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/mp4.c b/mp4.c index 479a2b69..02dd5a19 100644 --- a/mp4.c +++ b/mp4.c @@ -620,39 +620,6 @@ static int read_stsd(struct mp4 *f) return 1; } -static int32_t tag_add_field(struct mp4_metadata *meta, const char *item, - const char *value, int32_t len) -{ - meta->tags = para_realloc(meta->tags, - (meta->count + 1) * sizeof(struct mp4_tag)); - meta->tags[meta->count].item = para_strdup(item); - meta->tags[meta->count].len = len; - if (len >= 0) { - meta->tags[meta->count].value = para_malloc(len + 1); - memcpy(meta->tags[meta->count].value, value, len); - meta->tags[meta->count].value[len] = 0; - } else { - meta->tags[meta->count].value = para_strdup(value); - } - meta->count++; - return 1; -} - -static int read_string(struct mp4 *f, uint32_t length, char **result) -{ - char *str = para_malloc(length + 1); - int ret = read_data(f, str, length); - - if (ret <= 0) { - free(str); - *result = NULL; - } else { - str[length] = '\0'; - *result = str; - } - return ret; -} - static const char *get_metadata_name(uint8_t atom_type) { switch (atom_type) { @@ -669,9 +636,10 @@ static int parse_tag(struct mp4 *f, uint8_t parent, int32_t size) { int ret; uint64_t subsize, sumsize; - char *data = NULL; + char *value = NULL; uint32_t len = 0; uint64_t destpos; + struct mp4_tag *tag; for ( sumsize = 0; @@ -682,30 +650,43 @@ static int parse_tag(struct mp4 *f, uint8_t parent, int32_t size) uint8_t header_size = 0; ret = atom_read_header(f, &atom_type, &header_size, &subsize); if (ret <= 0) - return ret; + goto fail; destpos = get_position(f) + subsize - header_size; if (atom_type != ATOM_DATA) continue; ret = read_int8(f, NULL); /* version */ if (ret <= 0) - return ret; + goto fail; ret = read_int24(f, NULL); /* flags */ if (ret <= 0) - return ret; + goto fail; ret = read_int32(f, NULL); /* reserved */ if (ret <= 0) - return ret; - free(data); - ret = read_string(f, subsize - (header_size + 8), &data); - if (ret <= 0) - return ret; + goto fail; + ret = -ERRNO_TO_PARA_ERROR(EINVAL); + if (subsize < header_size + 8 || subsize > UINT_MAX) + goto fail; len = subsize - (header_size + 8); + free(value); + value = para_malloc(len + 1); + ret = read_data(f, value, len); + if (ret <= 0) + goto fail; + value[len] = '\0'; } - if (!data) - return -1; - tag_add_field(&f->meta, get_metadata_name(parent), data, len); - free(data); + if (!value) + return -ERRNO_TO_PARA_ERROR(EINVAL); + f->meta.tags = para_realloc(f->meta.tags, (f->meta.count + 1) + * sizeof(struct mp4_tag)); + tag = f->meta.tags + f->meta.count; + tag->item = para_strdup(get_metadata_name(parent)); + tag->value = value; + tag->len = len; + f->meta.count++; return 1; +fail: + free(value); + return ret; } static int read_mdhd(struct mp4 *f) @@ -780,7 +761,9 @@ static int32_t read_ilst(struct mp4 *f, int32_t size) case ATOM_ALBUM: case ATOM_COMMENT: case ATOM_DATE: - parse_tag(f, atom_type, subsize - header_size); + ret = parse_tag(f, atom_type, subsize - header_size); + if (ret <= 0) + return ret; } set_position(f, destpos); sumsize += subsize; @@ -807,9 +790,11 @@ static int32_t read_meta(struct mp4 *f, uint64_t size) return ret; if (subsize <= header_size + 4) return 1; - if (atom_type == ATOM_ILST) - read_ilst(f, subsize - (header_size + 4)); - else + if (atom_type == ATOM_ILST) { + ret = read_ilst(f, subsize - (header_size + 4)); + if (ret <= 0) + return ret; + } else set_position(f, get_position(f) + subsize - header_size); sumsize += subsize; } -- 2.39.2