[FFmpeg-cvslog] avfilter/formats: Fix heap-buffer overflow when merging channel layouts

[Andreas Rheinhardt]

ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt at gmail.com> | Thu Aug 13 04:02:26 2020 +0200| [4147f63d63358e5c1969bfe431ee08ca54f8434d] | committer: Andreas Rheinhardt

avfilter/formats: Fix heap-buffer overflow when merging channel layouts

The channel layouts accepted by ff_merge_channel_layouts() are of two
types: Ordinary channel layouts and generic channel layouts. These are
layouts that match all layouts with a certain number of channels.
Therefore parsing these channel layouts is not done in one go; instead
first the intersection of the ordinary layouts of the first input
list of channel layouts with the ordinary layouts of the second list is
determined, then the intersection of the ordinary layouts of the first
one and the generic layouts of the second one etc. In order to mark the
ordinary channel layouts that have already been matched as used they are
zeroed. The inner loop that does this is as follows:

for (j = 0; j < b->nb_channel_layouts; j++) {
    if (a->channel_layouts[i] == b->channel_layouts[j]) {
        ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
        a->channel_layouts[i] = b->channel_layouts[j] = 0;
    }
}

(Here ret->channel_layouts is the array containing the intersection of
the two input arrays.)

Yet the problem with this code is that after a match has been found, the
loop continues the search with the new value a->channel_layouts[i].
The intention of zeroing these elements was to make sure that elements
already paired at this stage are ignored later. And while they are indeed
ignored when pairing ordinary and generic channel layouts later, it has
the exact opposite effect when pairing ordinary channel layouts.

To see this consider the channel layouts A B C D E and E D C B A. In the
first round, A and A will be paired and added to ret->channel_layouts.
In the second round, the input arrays are 0 B C D E and E D C B 0.
At first B and B will be matched and zeroed, but after doing so matching
continues, but this time it will search for 0, which will match with the
last entry of the second array. ret->channel_layouts now contains A B 0.
In the third round, C 0 0 will be added to ret->channel_layouts etc.
This gives a quadratic amount of elements, yet the amount of elements
allocated for said array is only the sum of the sizes of a and b.

This issue can e.g. be reproduced by
ffmpeg -f lavfi -i anullsrc=cl=7.1 \
-af 'aformat=cl=mono|stereo|2.1|3.0|4.0,aformat=cl=4.0|3.0|2.1|stereo|mono' \
-f null -

The fix is easy: break out of the inner loop after having found a match.

Reviewed-by: Nicolas George <george at nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=4147f63d63358e5c1969bfe431ee08ca54f8434d
---

 libavfilter/formats.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index 3c7f099a94..efa2a39117 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -219,6 +219,7 @@ AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts *a,
             if (a->channel_layouts[i] == b->channel_layouts[j]) {
                 ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
                 a->channel_layouts[i] = b->channel_layouts[j] = 0;
+                break;
             }
         }
     }