[FFmpeg-devel] [PATCH 6/6] avfilter/af_surround: Check av_channel_layout_channel_from_index() stays within the fixed array used

[Michael Niedermayer]

On Sun, Jul 07, 2024 at 09:12:06PM +0200, Andreas Rheinhardt wrote:
> Andreas Rheinhardt:
> > Michael Niedermayer:
> >> Fixes: CID1516994 Out-of-bounds access
> >> Fixes: CID1516996 Out-of-bounds access
> >> Fixes: CID1516999 Out-of-bounds access
> >>
> >> Sponsored-by: Sovereign Tech Fund
> >> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >> ---
> >>  libavfilter/af_surround.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/libavfilter/af_surround.c b/libavfilter/af_surround.c
> >> index e37dddc3614..fab39a37ea9 100644
> >> --- a/libavfilter/af_surround.c
> >> +++ b/libavfilter/af_surround.c
> >> @@ -269,6 +269,9 @@ static int config_output(AVFilterLink *outlink)
> >>  
> >>      for (int ch = 0; ch < outlink->ch_layout.nb_channels; ch++) {
> >>          float iscale = 1.f;
> >> +        const int chan = av_channel_layout_channel_from_index(&s->out_ch_layout, ch);
> >> +        if (chan >= FF_ARRAY_ELEMS(sc_map))
> >> +            return AVERROR_PATCHWELCOME;
> >>  
> >>          ret = av_tx_init(&s->irdft[ch], &s->itx_fn, AV_TX_FLOAT_RDFT,
> >>                           1, s->win_size, &iscale, 0);
> > 
> > Can this happen?

IMHO, this doesnt matter. A filter that depends on a audio channel layout
API from another lib cannot depend on its implementation but just the
public API/ABI
So even if the av_channel_layout_* API didnt allow us to set such layout
today we would need to check for it

now can this happen?
try this:

./ffmpeg -i matrixbench_mpeg2.mpg -af surround=chl_out="123456789" -f null -

I get a
Segmentation fault (core dumped)

and it doesnt segfault after the patch


> > 
> 
> Apart from that: I think you are mistaken when you believe that this
> will "fix" the issue. Coverity will not think that these issues are
> fixed even with this check.

After this patch the issue is either detected as fixed or not,
if not then it becomes a false positive and either way is fixed

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240707/dffab785/attachment.sig>