Opened 5 years ago

Closed 5 years ago

#8226 closed defect (fixed)

A use-after-free bug in formats.c

Reported by: wurongxin Owned by:
Priority: normal Component: avfilter
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
How to reproduce:

% ffmpeg -i input ... output
ffmpeg version
built on ...

Patches should be submitted to the ffmpeg-devel mailing list and not this bug tracker.

In the source file libavfilter/formats.c, in the function "ff_set_common_samplerates", there is a potential double free bug. The variable "samplerates->formats" has been freed twice.

556.    int ff_set_common_samplerates(AVFilterContext *ctx,
557.                              AVFilterFormats *samplerates)
558.    {
559.        SET_COMMON_FORMATS(ctx, samplerates, in_samplerates, out_samplerates,
560.                           ff_formats_ref, ff_formats_unref, formats);
561.    }

The macro definition is as follows:

510.      #define SET_COMMON_FORMATS(ctx, fmts, in_fmts, out_fmts, ref_fn, unref_fn, list) \
511.        int count = 0, i;                                               \
512.                                                                        \
513.        if (!fmts)                                                      \
514.            return AVERROR(ENOMEM);                                     \
515.                                                                        \
516.        for (i = 0; i < ctx->nb_inputs; i++) {                          \
517.            if (ctx->inputs[i] && !ctx->inputs[i]->out_fmts) {          \
518.                int ret = ref_fn(fmts, &ctx->inputs[i]->out_fmts);      \
519.                if (ret < 0) {                                          \
520.                    unref_fn(&fmts);                                    \
521.                    av_freep(&fmts->list);                              \
522.                    av_freep(&fmts);                                    \
523.                    return ret;                                         \
524.                }                                                       \
525.                count++;                                                \
526.            }                                                           \
527.        }                                                               \
528.        for (i = 0; i < ctx->nb_outputs; i++) {                         \
529.            if (ctx->outputs[i] && !ctx->outputs[i]->in_fmts) {         \
530.                int ret = ref_fn(fmts, &ctx->outputs[i]->in_fmts);      \
531.                if (ret < 0) {                                          \
532.                    unref_fn(&fmts);                                    \
533.                    av_freep(&fmts->list);                              \
534.                    av_freep(&fmts);                                    \
535.                    return ret;                                         \
536.                }                                                       \
537.                count++;                                                \
538.            }                                                           \
539.        }                                                               \
540.                                                                        \
541.        if (!count) {                                                   \
542.            av_freep(&fmts->list);                                      \
543.            av_freep(&fmts->refs);                                      \
544.            av_freep(&fmts);                                            \
545.        }                                                               \
546.                                                                        \
547.        return 0;

As shown in the above definition, at Line 518, it will invoke the function "ff_formats_ref", and the variable fmts->formats will be freed. At Line 520, it will invoke the function "ff_formats_unref", and the variable fmts->formats will again be freed. This would lead to a double free bug.

To see how fmts->formats will be freed in the function "ff_formats_ref", please see the following code snippet. At Line 427, it will invoke the function "ff_formats_unref" and free the variable f->formats. It should be very important to be noted that, after the return of the function ff_formats_ref, the parameter "AVFilterFormats *f" will still point to the original memory location, although the "f" has been changed at Line 427.

440.    int ff_formats_ref(AVFilterFormats *f, AVFilterFormats **ref)
441.    {
442.       FORMATS_REF(f, ref, ff_formats_unref);
443.    }

...

419.    #define FORMATS_REF(f, ref, unref_fn)                                           \
420.        void *tmp;                                                                  \
421.                                                                                    \
422.        if (!f || !ref)                                                             \
423.            return AVERROR(ENOMEM);                                                 \
424.                                                                                    \
425.        tmp = av_realloc_array(f->refs, sizeof(*f->refs), f->refcount + 1);         \
426.        if (!tmp) {                                                                 \
427.            unref_fn(&f);                                                           \
428.            return AVERROR(ENOMEM);                                                 \
429.        }                                                                           \
430.        f->refs = tmp;                                                              \
431.        f->refs[f->refcount++] = ref;                                               \
432.        *ref = f;                                                                   \
433.        return 0
...

To see how the function "ff_formats_unref" frees the f->formats, please see the following code snippet. As we can see, at Line 469, according to the macro definition, "list" is "formats", and ref->formats will be freed at Line 469.

476.    void ff_formats_unref(AVFilterFormats **ref)
477.    {
478.        FORMATS_UNREF(ref, formats);
479.    }
...


455.    #define FORMATS_UNREF(ref, list)                                   \
456.    do {                                                               \
457.        int idx = -1;                                                  \
458.                                                                       \
459.        if (!*ref || !(*ref)->refs)                                    \
460.            return;                                                    \
461.                                                                       \
462.        FIND_REF_INDEX(ref, idx);                                      \
463.                                                                       \
464.        if (idx >= 0)                                                  \
465.            memmove((*ref)->refs + idx, (*ref)->refs + idx + 1,        \
466.                sizeof(*(*ref)->refs) * ((*ref)->refcount - idx - 1)); \
467.                                                                       \
468.        if(!--(*ref)->refcount) {                                      \
469.            av_free((*ref)->list);                                     \
470.            av_free((*ref)->refs);                                     \
471.            av_free(*ref);                                             \
472.        }                                                              \
473.        *ref = NULL;                                                   \
474.    } while (0)


Change History (2)

comment:1 by wurongxin, 5 years ago

Similar bug is seen at Line 568--572 in the function "ff_set_common_samplerates", and the root cause is exactly the same.

The possible patch for this bug could be inserted after Line 470 with the following code:

(*ref)->list = NULL;
(*ref)->refs = NULL;

Last edited 5 years ago by wurongxin (previous) (diff)

comment:2 by Elon Musk, 5 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.