Fixes: signed integer overflow: 6000 * -2147483648 cannot be represented in type 'int'
Fixes: 25700/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6578316302352384
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
get_content_url() allocates two buffers for temporary strings and when
one of them couldn't be allocated, it simply returns, although one of
the two allocations could have succeeded and would leak in this
scenario. This can be fixed by avoiding one of the temporary buffers.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
1. Perform the necessary reindentations after the last few commits.
2. Adapt switches to the ordinary indentation style.
3. Now that the effective lifetimes of the variables containing
the freshly allocated strings used when parsing the representation
are disjoint, the variables can be replaced by a single variable.
Doing so has the advantage of making it more clear that these are
throwaway variables, hence it has been done.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This allows to reduce the level of indentation for parsing the supported
representations (audio, video and subtitles). It also allows to avoid
some allocations and frees for unsupported representations.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit removes two always-true checks as well as a dead default
case of a switch. The check when parsing manifests is always true,
because we now jump to the cleaning code in case the format of the
representation is unknown. The default case of the switch is dead,
because the type of the representation is already checked at the
beginning of parse_manifest_representation(). The check when reading
the header is dead, because we error out if an error happened before.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the DASH demuxer used av_dynarray_add() to add
audio/video/subtitles representations to arrays. Yet av_dynarray_add()
frees the array upon failure, leading to leaks of its elements;
furthermore, the element to be added leaks, too.
This has been fixed by using av_dynarray_add_nofree() instead and by
freeing the elements that could not be added to the list. Furthermore,
errors from this are now checked and returned.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These languages are normally freed after having been added as metadata
to their respective AVStreams. Yet if one never reaches said point, they
leak. This can happen as a result of an error when reading the header or
as a result of refreshing the manifests.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The DASH demuxer currently extracts several strings at once from an xml
document before processing them one by one; these strings are allocated,
stored in local variables and need to be freed by the demuxer itself.
So if an error happens when processing one of them, all strings need to
be freed before returning. This has simply not been done, leading to
leaks.
A simple fix would be to add the necessary code for freeing; yet there is
a better solution: Avoid having several strings at the same time by
extracting a string, processing it and immediately freeing it. That way
one only has to free at most one string on error.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If parsing a representation fails, it is not added to the list of
representations and is therefore not freed in dash_close(); it therefore
leaked in most error paths in parse_manifest_representation() (some
error paths had (incomplete) code for freeing). This commit fixes
freeing the representation in this case.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
It is always zero. Also remove other unused elements.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
open_url() in the DASH as well in the hls demuxer share a common bug:
They modify an AVDictionary (i.e. set a new entry) given to them as
AVDictionary *, yet if this new entry leads to reallocation and
relocation of the AVDictionary, the caller's pointer will become
dangling, leading to use-after-frees. So pass an AVDictionary **.
(With the current implementation of AVDictionary the above can only
happen if the AVDictionary was empty initially (in which case the
new AVDictionary leaks); furthermore if the I/O is ordinary (i.e. opened
by avio_open2() or ffio_open_whitelist()), the dict is never empty (it
contains an rw_timeout entry from save_avio_options()). So this issue
could only happen if the caller sets a nondefault io_open callback, but
no AVIOContext (the AVFMT_FLAG_CUSTOM_IO flag won't be set in this
case). In case of the HLS demuxer, it was also necessary that setting
the "seekable" entry failed. Yet one should simply not rely on internals
of the AVDict API.)
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Just postpone the allocation of the dict until it is really needed
(after the checks that can fail).
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This currently doesn't cause any trouble, because the only caller did
not clean up the representation upon error at all; but fixing this is
a prerequisite for doing so.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The code in question seems to have been copied from about 70 lines
above; yet the code here is only executed if some of the variables
(namely representation_segmenttemplate_node and fragment_template_node)
are NULL, so it makes no sense to check them for a child element.
Also remove a redundant resetting of a pointer to an AVFormatContext
after avformat_close_input() (which already sets the pointer to NULL).
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When using one of the AV_DICT_DONT_STRDUP_KEY/VAL flags, av_dict_set()
already frees the key/value on error, so that freeing it again would
lead to a double free.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The AAX demuxer reads a 32bit number containing the amount of entries
of an array and stores it in an uint32_t. Yet when iterating over this
array, a loop counter of type int is used. This leads to undefined
behaviour if the amount of entries is not in the range of int; to avoid
this, it is generally good to use the same type for the loop counter as
for the variable it is compared to. This is done in one of the two loops
affected by this.
In the other loop, the undefined behaviour can begin even earlier: Here
the loop counter is multiplied by an uint16_t which can overflow as soon
as the loop counter is > 2^15. Using an unsigned type would avoid the
undefined behaviour, but truncation would still be possible, so use an
uint64_t.
Also use an uint32_t for a variable containing an index in said array.
This fixes Coverity issue #1466767.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
There was almost no overlap between them: The only field used by both
was an int named samples_per_frame. Therefore this commit separates
them.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The tedcaptions demuxer uses an AVBPrint whose string is not restricted
to its internal buffer; it therefore needs to be cleaned up, yet this is
not done on error, as parse_file() returned simply returned directly.
This is fixed by going to fail first in such cases.
Furthermore, there is also a second way how this string can leak: By
having more than one subtitle per subtitle block, as the new one simply
overwrites the old one in this case as the AVBPrint is initialized each
time upon encountering a subtitle line. The code has been modified to
simply append the new subtitle to the old one, so that the old one can't
leak any more.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Turns out that there are files with multiple (reasonably-sized) BASF
blocks. Some of the files just have particularly large frames (~10s).
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
Enforcing num_chunks == 1 only makes sense when demuxing from an ASF
file. When embedded in a BRP file, an ASF stream can have multiple chunks.
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
The SWF muxer accepts at most one mp3 audio and at most one VP6F, FLV1
or MJPEG stream. Upon encountering an mp3 stream, a fifo is allocated
that leaks if one of the subsequent streams is incompliant with the
restrictions mentioned above or if the framerate or samplerate are
invalid. This is fixed by adding a deinit function to free said fifo.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Before patch, fate test for dnn may fail in some Windows environment
while succeed in my Linux. The bug was caused by a wrong loop boundary.
After patch, fate test succeed in my windows mingw 64-bit.
Signed-off-by: Xu Jun <xujunzz@sjtu.edu.cn>
Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
The RealMedia demuxer uses the priv_data of its streams to store a
structure containing an AVPacket. These packets are unreferenced in the
read_close function, yet said function simply presumed that the
priv_data has been successfully allocated. This implies that it mustn't
be called when an allocation of priv_data fails; but this can happen
since commit 35bbc1955a if one has a
stream with multiple substreams (also exported as AVStream) and if
allocating the priv_data for one of these substreams fails.
This has been fixed by making sure that read_close can handle the case
in which priv_data has not been successfully allocated.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The RealMedia demuxer's read_header function initially initializes ret,
the variable designated for the return variable to -1. Afterwards, chunks
of the file are parsed in a loop until an error happens or until the actual
frame data is encountered. If the first function whose return
value is put into ret doesn't fail, then ret contains a value >= 0
(actually == 0) and this is what will be returned if an error is
encountered afterwards.
This is a regression since 35bbc1955a.
Before that, ret had never been overwritten with a nonnegative value.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Do this by only keeping the only function pointer from the
AVFloatDSPContext that is needed lateron. This also allows to remove the
decoders' close function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Michael Niedermayer
Andreas Rheinhardt
Limin Wang
Lou Logan
Zhao Zhili
Zane van Iperen
Xu Jun