Fixes: out of array access
Fixes: 24823/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4855119863349248
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes issue reported by: Xu Guangxin <guangxin.xu@intel.com>
Original report:
Steps to reproduce:
1. ./configure --enable-debug=3 --disable-libx264 && make install
2. ffmpeg -i input.mp4 -profile:v baseline output.mp4 -y
you will see a crash like this:
[mpeg4 @ 0x5555575854c0] [Eval @ 0x7fffffffbf80] Undefined constant or missing '(' in 'baseline'
[mpeg4 @ 0x5555575854c0] Unable to parse option value "baseline"
[mpeg4 @ 0x5555575854c0] Error setting option profile to value baseline.
Thread 1 "ffmpeg" received signal SIGSEGV, Segmentation fault.
root cause:
If the codec has FF_CODEC_CAP_INIT_CLEANUP flag, and avcodec_open2 got an error before avctx->codec->init,
the ff_mpv_encode_end will face a null s->avctx.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Here it even leads to the complete removal of the context.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Winnov WNV1 format is designed for a little-endian bitstream reader;
yet our decoder reversed every byte bitwise (in a buffer only
allocated for this purpose) to use a big-endian bitstream reader. This
commit stops this.
Two things needed to be done to achieve this: The codes in the table used
to initialize a VLC reader needed to be reversed bitwise (when
initializing a VLC in LE mode, it is expected that the first bit to be
read is in the least significant bit; with BE codes the first bit to be
read is the most significant bit of the code) and the following
expression needed to be adapted:
ff_reverse[get_bits(&w->gb, 8 - w->shift)]
But this is easy: When only the bits read are reversed, they coincide
with what a little-endian bitstream reader reads that reads the
original, not-reversed data. But ff_reverse always reverses the full
eight bits and this also performs a shift by (8 - (8 - w->shift)) on top
of reversing the bits read. So the above line needs to be changed to
get_bits(&w->gb, 8 - w->shift) << w->shift
and this also shows why the variable shift is named the way it is.
Finally, this also fixes a hypothetical memleak: For gigantic packets,
initializing a GetBitContext can fail and in this case, the buffer
containing the reversed data would leak.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The frame will only be allocated a few lines below.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
TrueMotion 2.0 uses Huffmann trees. To parse them, the decoder allocates
arrays for the codes, their lengths and their value; afterwards a VLC
table is initialized using these values. If everything up to this point
succeeds, a new buffer of the same size as the already allocated arrays
for the values is allocated and upon success the values are copied into
the new array; all the old arrays are then freed. Yet if allocating the
new array fails, the old arrays get freed, but the VLC table doesn't.
This leak is fixed by not allocating a new array at all; instead the old
array is simply reused, ensuring that nothing can fail after the
creation of the VLC table.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
MXF CDCI color range was being set to (1<<sc->component_depth) - 1
for full range but it should be (1<<sc->component_depth) as 0 is
a valid value.
Signed-off-by: Harry Mallon <harry.mallon@codex.online>
if taken from stack, they may have garbage in the upper bits otherwise.
Also, there are only 8 arguments, so don't attempt to load 11.
Fixes SIGSEV crashes in some targets.
Reviewed-by: durandal_1707
Signed-off-by: James Almer <jamrial@gmail.com>
A few popular sites have started generating MP4 files which have a
sidx plus an mfra. The sidx accounts for all size except the mfra,
so the old code did not mark the fragment index as complete.
Instead we can just check if there's an mfra and if its size makes
up the difference we can mark the index as complete.
Bug: https://crbug.com/1107130
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
In case the multichannel HRIR mode was enabled, an error could happen
between allocating a channel layouts list and attaching it to its target
destination. If an error happened, the list would leak. This is fixed by
attaching the list to its target directly after its allocation.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter uses a variable number of inpads and allocates them
in its init function; if all goes well, the number of inpads coincides
with a number stored in the filter's private context. Yet if allocating a
subsequent inpad fails, the uninit function nevertheless uses the number
stored in the private context to determine the number of inpads to free
and not the AVFilterContext's nb_inputs. This will lead to an access
beyond the end of the allocated AVFilterContext.input_pads array and
an invalid free.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The signature filter uses qsort, but its compare function doesn't have
the signature required of such a function; therefore it casts the
function pointer to void. Yet this is wrong:
C90 only guarantees that one can convert a pointer to any incomplete
type or object type to void* and back with the result comparing equal
to the original which makes pointers to void generic pointers to
incomplete or object type. Yet C90 lacks a generic function pointer
type.
C99 additionally guarantees that a pointer to a function of one type may
be converted to a pointer to a function of another type with the result
and the original comparing equal when converting back.
This makes any function pointer type a generic function pointer type.
Yet even this does not make pointers to void generic function pointers.
Both GCC and Clang emit warnings for this when in pedantic mode.
This commit fixes this by modifying the compare function to comply with
the expected signature.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If an error happens between allocating a string intended to be used as
an inpad's name and attaching it to its input pad, the string leaks.
Fix this by inserting the inpad directly after allocating its string.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If the names are always the same, they need not be duplicated; doing so
saves allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The aiir filter adds output pads in its init function. Each of these
output pads had a name which was allocated and to be freed in the uninit
function. Given that the aiir filter has between one and two outputs,
one output pad's name was freed unconditionally and a second was freed
conditionally.
Yet if adding output pads fails, there are no output pads at all and
trying to free a nonexistent pad's name will lead to a segfault.
Furthermore, if the name could be successfully allocated, yet adding the
new pad fails, the name would leak.
This commit fixes this by not allocating the pads' names at all any
more: They are constant anyway. This allows to remove the code to free
them and hence fixes the aforementioned bugs.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are always the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are always the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are mostly the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are always the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are always the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names leak because freeing them in the uninit function has been
forgotten. Instead of adding the freeing code, this commit stops
allocating these names. They are constants anyway.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These names are always the same, so not using duplicates saves
allocations, checks for the allocations as well as frees.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
It has been forgotten to free the name of the second outpad if attaching
the first one to the AVFilterContext fails. Fixing this is easy: Only
prepare the second outpad after (and if) the first outpad has been
successfully attached to the AVFilterContext.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fixes: out of array access
Fixes: 24825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-6326925027704832
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
