Andreas Rheinhardt:
> Up until now, using NULL as key in av_dict_get() on a non-empty
> AVDictionary would crash; using NULL as key in av_dict_set()
> would also crash for a non-empty AVDictionary unless AV_DICT_MULTIKEY
> was set; in case the dictionary was initially empty or AV_DICT_MULTIKEY
> was set, it was even possible for av_dict_set() to succeed when
> adding a NULL key, namely when one uses a value != NULL and
> the AV_DICT_DONT_STRDUP_VAL flag. Using av_dict_get() on such
> an AVDictionary will usually lead to crashes, though.
>
> Fix this by actually checking for key in both functions; error out
> if they are NULL.
>
> While just at it, also stop relying on av_strdup(NULL) to return NULL
> in av_dict_set().
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
> ---
> libavutil/dict.c | 27 +++++++++++++++++----------
> libavutil/tests/dict.c | 12 +++++++++---
> 2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 4bba041d0a..14ad780a79 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -43,7 +43,7 @@ AVDictionaryEntry *av_dict_get(const AVDictionary *m, const
> char *key,
> {
> unsigned int i, j;
>
> - if (!m)
> + if (!m || !key)
> return NULL;
>
> if (prev)
> @@ -74,7 +74,16 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> AVDictionary *m = *pm;
> AVDictionaryEntry *tag = NULL;
> char *copy_key = NULL, *copy_value = NULL;
> + int err;
>
> + if (flags & AV_DICT_DONT_STRDUP_VAL)
> + copy_value = (void *)value;
> + else if (value)
> + copy_value = av_strdup(value);
> + if (!key) {
> + err = AVERROR(EINVAL);
> + goto err_out;
> + }
> if (!(flags & AV_DICT_MULTIKEY)) {
> tag = av_dict_get(m, key, NULL, flags);
> }
> @@ -82,14 +91,10 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> copy_key = (void *)key;
> else
> copy_key = av_strdup(key);
> - if (flags & AV_DICT_DONT_STRDUP_VAL)
> - copy_value = (void *)value;
> - else if (copy_key)
> - copy_value = av_strdup(value);
> if (!m)
> m = *pm = av_mallocz(sizeof(*m));
> - if (!m || (key && !copy_key) || (value && !copy_value))
> - goto err_out;
> + if (!m || !copy_key || (value && !copy_value))
> + goto enomem;
>
> if (tag) {
> if (flags & AV_DICT_DONT_OVERWRITE) {
> @@ -103,7 +108,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> size_t len = oldlen + new_part_len + 1;
> char *newval = av_realloc(tag->value, len);
> if (!newval)
> - goto err_out;
> + goto enomem;
> memcpy(newval + oldlen, copy_value, new_part_len + 1);
> av_freep(©_value);
> copy_value = newval;
> @@ -115,7 +120,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> AVDictionaryEntry *tmp = av_realloc_array(m->elems,
> m->count + 1,
> sizeof(*m->elems));
> if (!tmp)
> - goto err_out;
> + goto enomem;
> m->elems = tmp;
> }
> if (copy_value) {
> @@ -132,6 +137,8 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
>
> return 0;
>
> +enomem:
> + err = AVERROR(ENOMEM);
> err_out:
> if (m && !m->count) {
> av_freep(&m->elems);
> @@ -139,7 +146,7 @@ err_out:
> }
> av_free(copy_key);
> av_free(copy_value);
> - return AVERROR(ENOMEM);
> + return err;
> }
>
> int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value,
> diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
> index 56e98557a7..d053545f4d 100644
> --- a/libavutil/tests/dict.c
> +++ b/libavutil/tests/dict.c
> @@ -91,14 +91,20 @@ int main(void)
> av_dict_set(&dict, "f", NULL, 0);
> av_dict_set(&dict, "ff", "f", 0);
> av_dict_set(&dict, "ff", "f", AV_DICT_APPEND);
> + if (av_dict_get(dict, NULL, NULL, 0))
> + printf("av_dict_get() does not correctly handle NULL key.\n");
> e = NULL;
> while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> printf("%s %s\n", e->key, e->value);
> av_dict_free(&dict);
>
> - av_dict_set(&dict, NULL, "a", 0);
> - av_dict_set(&dict, NULL, "b", 0);
> - av_dict_get(dict, NULL, NULL, 0);
> + if (av_dict_set(&dict, NULL, "a", 0) >= 0 ||
> + av_dict_set(&dict, NULL, "b", 0) >= 0 ||
> + av_dict_set(&dict, NULL, NULL, AV_DICT_DONT_STRDUP_KEY) >= 0 ||
> + av_dict_set(&dict, NULL, av_strdup("b"), AV_DICT_DONT_STRDUP_VAL) >=
> 0 ||
> + av_dict_count(dict))
> + printf("av_dict_set does not correctly handle NULL key\n");
> +
> e = NULL;
> while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> printf("'%s' '%s'\n", e->key, e->value);
Will apply this tonight unless there are objections.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
