• Alexey Tourbin's avatar
    avfilter/af_stereowiden: fix read/write past the end of buffer · 906ee411
    Alexey Tourbin authored
    The stereowiden filter uses a buffer, s->buffer[], and a pointer
    within the buffer, s->write, to implement inter-channel delays.
    The loop which applies the delayed samples turns out to be faulty.
    
       109      for (n = 0; n < in->nb_samples; n++, src += 2, dst += 2) {
       110          const float left = src[0], right = src[1];
       111          float *read = s->write + 2;
       112
       113          if (read > s->buffer + s->length)
       114              read = s->buffer;
       115
       116          dst[0] = drymix * left - crossfeed * right - feedback * read[1];
       117          dst[1] = drymix * right - crossfeed * left - feedback * read[0];
       118
       119          s->write[0] = left;
       120          s->write[1] = right;
       121
       122          if (s->write == s->buffer + s->length)
       123              s->write = s->buffer;
       124          else
       125              s->write += 2;
       126      }
    
    For one, the buffer gets written past its end in lines 119-120, before
    the bound check is done in lines 122-123.  This can be easily confirmed
    by valgrind.
    
    ==3544== Invalid read of size 4
    ==3544==    at 0x593B41: filter_frame (af_stereowiden.c:116)
    ==3544==  Address 0xb1b03c4 is 4 bytes after a block of size 7,680 alloc'd
    ==3544==
    ==3544== Invalid read of size 4
    ==3544==    at 0x593B66: filter_frame (af_stereowiden.c:117)
    ==3544==  Address 0xb1b03c0 is 0 bytes after a block of size 7,680 alloc'd
    ==3544==
    ==3544== Invalid write of size 4
    ==3544==    at 0x593B79: filter_frame (af_stereowiden.c:119)
    ==3544==  Address 0xb1b03c0 is 0 bytes after a block of size 7,680 alloc'd
    ==3544==
    ==3544== Invalid write of size 4
    ==3544==    at 0x593B7D: filter_frame (af_stereowiden.c:120)
    ==3544==  Address 0xb1b03c4 is 4 bytes after a block of size 7,680 alloc'd
    
    Also, using two separate pointers, s->write and read = s->write + 2,
    does not seem to be well thought out.  To apply the delay of s->buffer[],
    it is enough to read the delayed samples at the current position within
    the buffer, and then to store new samples at the same current position.
    Thus the application of delayed samples can probably be best described
    with a single pointer s->cur.
    
    I also introduce a minor change to ensure that the size of s->buffer[]
    is always a multiple of 2.  Since the delay parameter is a float, it is
    otherwise possible to trick the code into allocating off-by-one buffer.
    906ee411
Name
Last commit
Last update
compat Loading commit data...
doc Loading commit data...
libavcodec Loading commit data...
libavdevice Loading commit data...
libavfilter Loading commit data...
libavformat Loading commit data...
libavresample Loading commit data...
libavutil Loading commit data...
libpostproc Loading commit data...
libswresample Loading commit data...
libswscale Loading commit data...
presets Loading commit data...
tests Loading commit data...
tools Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.travis.yml Loading commit data...
COPYING.GPLv2 Loading commit data...
COPYING.GPLv3 Loading commit data...
COPYING.LGPLv2.1 Loading commit data...
COPYING.LGPLv3 Loading commit data...
CREDITS Loading commit data...
Changelog Loading commit data...
INSTALL.md Loading commit data...
LICENSE.md Loading commit data...
MAINTAINERS Loading commit data...
Makefile Loading commit data...
README.md Loading commit data...
RELEASE Loading commit data...
arch.mak Loading commit data...
cmdutils.c Loading commit data...
cmdutils.h Loading commit data...
cmdutils_common_opts.h Loading commit data...
cmdutils_opencl.c Loading commit data...
common.mak Loading commit data...
configure Loading commit data...
ffmpeg.c Loading commit data...
ffmpeg.h Loading commit data...
ffmpeg_cuvid.c Loading commit data...
ffmpeg_dxva2.c Loading commit data...
ffmpeg_filter.c Loading commit data...
ffmpeg_opt.c Loading commit data...
ffmpeg_qsv.c Loading commit data...
ffmpeg_vaapi.c Loading commit data...
ffmpeg_vdpau.c Loading commit data...
ffmpeg_videotoolbox.c Loading commit data...
ffplay.c Loading commit data...
ffprobe.c Loading commit data...
ffserver.c Loading commit data...
ffserver_config.c Loading commit data...
ffserver_config.h Loading commit data...
library.mak Loading commit data...
version.sh Loading commit data...