Fixes: out of array access
Fixes: division by zero
Fixes: vdfZuLhrGLdR
Found-by: zhang xingxing
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 590fb407b0)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array access
Fixes: Af5rtPnOunVT
Found-by: Umar Pathan (Umar0x)
Signed-off-by: Umar Pathan <hello@umar.ac>
(cherry picked from commit a809a784ec)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array write
Found-by: Michael Niedermayer with Forgejo Fairy
(cherry picked from commit 68845e26f7)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array write
Fixes: 9vZSLhhnp3ps
Found-by: Kai Martin, KG3N Research
(cherry picked from commit e058af88ab)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: invalid free
Fixes: 6Qd6MOq0EvLi
Found-by: Gabe Sherman (Trail of Bits)
(cherry picked from commit 094a2f8a2a)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
At the time that this code was introduced into FFmpeg in April 2022 by commit
b9a25963f7, it was correct as written. The latest release of
lcms2 at the time was 2.13.1 in which `cmsDeleteContext(NULL)` was a no-op.
Since lcms2 version 2.15, released Mar 2023, the same call is no longer a no-op and
instead means destroy the global context. The new behavior is justifiable as being
consistent with the rest of lcms2's API, where passing NULL for a cmsContext argument
means operate on the global context. For example `cmsDupContext(NULL, ...)` constructs a
context duplicated from the global context; `cmsPluginTHR(NULL, ...)` registers a plugin
into the global context.
The upstream commit that introduced the behavior change for `cmsDeleteContext` is:
a9e4601ceb
In FFmpeg, the new behavior leads to a memory leak if `ff_icc_context_uninit` is ever
called on an `FFIccContext` without `ff_icc_context_init`. Here is why that can happen,
and here is how it leads to a memory leak.
Naively, uninitializing something which was never initialized seems like it shouldn't
happen, but in this case these contexts are *lazily* initialized and *unconditionally*
deinitialized. `FFIccContext` is a member of `AVCodecInternal`:
// libavcodec/internal.h:148-150
#if CONFIG_LCMS2
FFIccContext icc; /* used to read and write embedded ICC profiles */
#endif
It is initialized conditionally by the snippet of `detect_colorspace` shown below, or by
similary shaped code in libavcodec/encode.c. The `avci->icc.avctx` guards "has it already
been initialized". The early returns beforehand are codepaths for which `icc` will not be
initialized, and will remain all-zero as allocated by `av_mallocz` in
`ff_decode_internal_alloc`.
// libavcodec/decode.c:534-542
if (!(avctx->flags2 & AV_CODEC_FLAG2_ICC_PROFILES))
return 0;
sd = av_frame_get_side_data(frame, AV_FRAME_DATA_ICC_PROFILE);
if (!sd || !sd->size)
return 0;
if (!avci->icc.avctx) {
ret = ff_icc_context_init(&avci->icc, avctx);
Later, `avci->icc` is uninitialized unconditionally by `ff_codec_close`:
// libavcodec/avcodec.c:447-480
if (avcodec_is_open(avctx)) {
...
#if CONFIG_LCMS2
ff_icc_context_uninit(&avci->icc);
#endif
So every codec closed without `AV_CODEC_FLAGS2_ICC_PROFILES`, or without an ICC side-data
packet, reaches `ff_icc_context_uninit` on a never-initialized context.
As part of `cmsDeleteContext(NULL)` destroying lcms2's global context, it runs
`cmsUnregisterPlugins()` and destroys `globalContext.MemPool`. Since the global context is
shared by every user of lcms2 in the process, closing an FFmpeg codec silently tears down
unrelated libraries' lcms2 state. The most visible symptom is that
`_cmsRegisterMutexPlugin(NULL, NULL)` nulls out the mutex callbacks, after which
`cmsCloseProfile()` leaks each profile's `UsrMutex` and profiles opened later are left
without any locking at all.
The fix is to delete only contexts created by FFmpeg.
Signed-off-by: David Tolnay <dtolnay@gmail.com>
(cherry picked from commit 705286a8a7)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Affects filters like vf_tonemap, vf_nlmeans_vulkan and vf_zscale that use
NAN as an explicit default value to indicate "not set by user".
Signed-off-by: Niklas Haas <git@haasn.dev>
(cherry picked from commit 5815e6343c)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Reject negative recording durations for input and output -t before they reach streamcopy or trim handling. Keep -t 0 accepted.
Signed-off-by: 张永鹏 <roc63@outlook.com>
(cherry picked from commit 629a5b4b0f)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
When the first slice of a picture is skipped, an undecodable RASL
after a seek or avctx->skip_frame, the remaining slices were still
parsed against the stale context and failed the PPS change, dependent
slice, or missing first slice checks, logging a decoding error per
slice. Drop them before parsing instead.
Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
(cherry picked from commit e5eb7581dd)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
In VVC, there are two schemes for addressing reference pictures in
the DPB:
1. Normally, the POC of the reference picture is signaled using a delta
from the current picture POC.
2. In the case of long-term reference pictures (LTRPs), which may be far
away from the current POC, the absolute value of the LTRP POC is
signalled. This scheme has two sub-schemes:
a. Signalling the entirety of the absolute value of the LTRP
POC would be expensive, so in most cases we only signal the
sps_log2_max_pic_order_cnt_lsb_minus4 + 4 least-significant bits
of the LTRP POC.
b. In the case of a collision in the least-significant bits,
the full POC is signalled.
Both schemes are implemented in find_ref_idx. The use_msb parameter
of find_ref_idx is nonzero in the case of schemes 1 and 2b, and zero in
the case of scheme 2a.
This commit fixes a bug in scheme 2a. The issue arises from the fact
that the current picture is stored in the DPB at the time find_ref_idx
is invoked in FFmpeg's implementation, whereas in the specification the
current picture is not yet considered part of the DPB. The current
picture may have a POC which shares least-significant bits with an LTRP.
The specification does not identify this as a collision however (see the
condition in 7.4.10, under the semantics of
delta_poc_msb_cycle_present_flag), and so scheme 2a is used rather than
scheme 2b. If the current frame precedes the correct LTRP in the DPB,
FFmpeg's implementation then interprets the signalling to mean a frame
which references itself. The problematic callsite identifies this as an
issue and returns an AVERROR_INVALIDDATA, but this scenario can occur in
valid bitstreams. For example, the conformance bitstream
LTRP_A_ERICSSON_3 has one such instance. FFmpeg gets away with decoding
it currently because the LTRP precedes the current picture in the DPB,
but this is by chance rather than design.
The commit addresses this issue by skipping over the current picture
inside the main loop of find_ref_idx. An unambiguous reference to the
current picture is still syntactically possible but illegal when using
scheme 2b. The handling of this case is moved from find_ref_idx to the
caller, which allows for simplifying find_ref_idx.
Signed-off-by: Frank Plowman <post@frankplowman.com>
(cherry picked from commit 2b3609ab61)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The VC-1 mc prediction for combined horizontal and vertical offset
involves first a vertical 4-tap filter, followed by shifting,
followed by a horizontal 4-tap filter, followed by shifting by 7.
Both passes involve negative coefficients and therefore the
intermediate values for the >> 7 can exceed the range of
0..255*128 a bit in both directions. Therefore one can not simply
use either a logical or an arithmetic right shift. But one can use
them after having added a certain bias to ensure that one is in
the proper range. This is what this commit does.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
(cherry picked from commit 0056dd32fd)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Broken in ba56a300a9,
ba56a300a9 and again
in 38b8ae4112.
This means that no has compiled git master on mips
since 2024-06-04. And no one complained.
Hint: One still needs to pass -flax-vector-conversions
here in order to compile this file.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
(cherry picked from commit d5b8439bdd)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The P- and B-picture per-MB guards added in 2a8785966c bail out of the loop
to error concealment when get_bits_left() <= 1, but P- and B-picture MBs can
legitimately cost exactly 1 bit in certain cases. This patch relaxes the guards
to accommodate those cases.
## Explanation
With the SKIPMB bitplane in Raw mode a skipped macroblock is a single SKIPMBBIT
(SMPTE ST 421:2013, 7.1.3.7, p.109) and then "may only contain the HYBRIDPRED
syntax element" (8.3.4.4, p.140), which is only read when predictors A and C
are in-bounds and the predicted vector differs from A or C by more than 32
quarter-pels (8.3.5.3.5, Figure 55, pp.149-150). That condition isn't met in a
static region so such a MB costs just 1 bit. A direct+skipped B-picture MB
likewise reads nothing beyond its raw flag (Table 30, p.86).
In Simple/Main profile a picture is byte-aligned with no stop bit, so if a
picture ends with a 1-bit skipped macroblock and its last coded bit lands on a
byte boundary, the final MB is reached with exactly one bit left. The guard
then conceals a valid macroblock, sets FF_DECODE_ERROR_CONCEALMENT_ACTIVE
on an undamaged frame, and the damage propagates to every frame up to the next
keyframe. (Advanced Profile BDUs end in a trailing '1' bit per Annex E.2/E.3
p.437, so they always have at least two bit left and are therefore unaffected.)
## Reproducing
Reproducible with an existing sample (prior to this patch):
$ ffmpeg -i https://samples.ffmpeg.org/V-codecs/WMV9/wmv-samples/sample-wmv9-simple-SP@ML.wmv -map 0:v -f null - 2>&1 | grep -A1 concealing
[wmv3 @ 0x122605430] concealing 75 DC, 75 AC, 75 MV errors in P frame
[vist#0:0/wmv3 @ 0x122604080] [dec:wmv3 @ 0x122604df0] corrupt decoded frame
[wmv3 @ 0x122605430] concealing 50 DC, 50 AC, 50 MV errors in P frame
[vist#0:0/wmv3 @ 0x122604080] [dec:wmv3 @ 0x122604df0] corrupt decoded frame
[wmv3 @ 0x122605430] concealing 100 DC, 100 AC, 100 MV errors in P frame
[vist#0:0/wmv3 @ 0x122604080] [dec:wmv3 @ 0x122604df0] corrupt decoded frame
## References
- SMPTE ST 421:2013 spec: https://pub.smpte.org/doc/st421/20131008-pub/
Signed-off-by: J. Brandt Buckley <brandt@runlevel1.com>
(cherry picked from commit ec8411e854)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
ef167512ab changed to parse RPU forward as far as we can go, but also
removed zero padding trimming, so the ext block size check on files with
big padding could trigger now. Restore the trailing zero strip. Note
that this does care only about zero padded NALs, while we have seen some
files with garbage padding, they are not considered here.
The only real way to find the end of RPU is to compute CRC32, as the
0x80 byte may be ambiguous. But we don't want that, and most (all) files
don't need that.
Also adjust the ext block remaining size check to minimal possible
(block with 0 size).
Fixes: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/24428
Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
(cherry picked from commit 22b96de803)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
fix issue: issues/23057 POC4
In refresh_manifest(), when timelines are present, fragment_timescale is
used as the denominator to compute currentTime. If fragment_timescale is
zero (e.g., timescale attribute missing from a crafted manifest with
SegmentTimeline), this results in a division by zero.
Add fragment_timescale > 0 guard to both the video and audio timeline
processing paths to prevent the crash.
Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
(cherry picked from commit 6a351a3113)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
av_encryption_init_info_alloc() only allocates key_ids when key_id_size is
non-zero, but stores num_key_ids regardless. If num_key_ids is non-zero and
key_id_size is 0, the returned object has a NULL key_ids array alongside a
non-zero num_key_ids. av_encryption_init_info_free(),
av_encryption_init_info_get_side_data() and
av_encryption_init_info_add_side_data() then loop over num_key_ids and
dereference key_ids[i], leading to a null pointer dereference.
Reject the num_key_ids > 0 && key_id_size == 0 combination instead of
returning an inconsistent object.
Signed-off-by: tangsha <tangsha@kylinos.cn>
(cherry picked from commit 9fc8c785e2)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
the genh demuxer had the same short-read issue at one location. the read is now performed with `ffio_read_size()` so truncated input is rejected instead of leaving uninitialized data in the extradata.
`ff_alloc_extradata()` also had its return value unchecked. if allocation failed, the code could dereference a null pointer. the return value is now checked.
the affected path is capped at two channels, so the potential information leak is relatively small, but it is the same underlying bug class.
Signed-off-by: unfunnyatearug <yalisib2222@gmail.com>
(cherry picked from commit 4b47405e1c)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
In buf_realloc, os->bufsize is unsigned int and the code doubles it:
nb = av_realloc(os->buf, 2*os->bufsize + AV_INPUT_BUFFER_PADDING_SIZE);
When os->bufsize exceeds UINT_MAX/2, the multiplication overflows
to a small value, av_realloc returns a tiny buffer, and the subsequent
page write in ogg_read_page goes out of bounds.
Reject the reallocation before the overflow can occur. The largest
safe value is (UINT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) / 2.
Signed-off-by: age5000 <age5000@proton.me>
(cherry picked from commit d7d119bbe2)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The tile count limit was only checked once on entry, but tileCount is
incremented by ff_vk_decode_add_slice() inside the loop, so a stream
carrying more than MAX_TILES tiles writes past the end of the
MAX_TILES-element tile_sizes array.
Check the limit on each iteration, and reject at MAX_TILES rather than
above it, since tileCount is used as the write index.
(cherry picked from commit d29ff88422)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Found during triage of the security report
Fixes: NULL pointer dereference
Fixes: 5cNIJSj1u1Lc/input-format-after-header.bin / gen-format-after-header.py
Fixes: 5cNIJSj1u1Lc
(cherry picked from commit 400912007a)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Found during review of the fix for the security report
Fixes: NULL pointer dereference
Fixes: 5cNIJSj1u1Lc/input-frameindex-path.bin / gen-frameindex-path.py
Fixes: 5cNIJSj1u1Lc
(cherry picked from commit d66ee0411c)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
mov_read_keys() allocated (count + 1) pointers from the 32 bit entry
count of the keys atom before reading a single key record, so a 56 byte
file declaring 16777216 keys made ffprobe allocate and zero 128 MiB.
Every key record takes at least 8 bytes, so a count above atom.size / 8
cannot be satisfied and is rejected before the allocation.
Fixes: unbounded allocation
Fixes: poc-keys-count-16777216.mp4
Fixes: hKO4f7GYoUd0
Found-by: He Huang, discovery assisted by NexusSan
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 67926e6667)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: reallocation of the IBC virtual buffers on every frame
(cherry picked from commit 5c161e1a62)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: division by zero
Fixes: tiny_16x8.h264
Fixes: W9IyRhjCcQxd
Found-by: zhang xingxing
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 2c05ce65e1)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
automated reviews otherwise treat this flag as a argument to
demand that any random subset of frames can be skiped and still
all extract their parameters correctly
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 3369578ba4)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
vsnprintf() returns the number of characters that would have been written
when its output is truncated. Passing that value to avio_write() makes it
read beyond the 128-byte stack buffer for long metadata values.
Use avio_vprintf() to write the complete formatted string without the fixed
intermediate buffer.
Fixes: b0159af6bc
Signed-off-by: Yongqiang Tian <yqtian668@gmail.com>
(cherry picked from commit fd625f452c)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
For IP-literal hosts mbedtls_ssl_set_hostname() was skipped, so a
CA-trusted certificate for an unrelated name was accepted with no CN/SAN
check. Set the name for numeric hosts too; mbedTLS matches IP literals
against iPAddress SANs.
Fixes: certificate host mismatch accepted for IP-literal hosts (CWE-297)
Fixes: oLS26nsaOqUr
Found-by: Joshua Rogers <joshua.rogers@aisle.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 57a2e704b4)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Process only complete 16-pixel blocks directly into the destination.
This makes the previously unreachable scratch-buffer tail handle the
remaining pixels. Remove its rewind and use the destination pixel stride
for both the copy offset and length.
Signed-off-by: Hao Chen <haochenc@google.com>
(cherry picked from commit 884590dd4a)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Found-by: Umar Pathan (Umar0x)
Signed-off-by: Umar Pathan <hello@umar.ac>
Fixes: out of array access
Fixes: signed integer overflow: 2147482624 + 1024 cannot be represented in type 'int'
Fixes: hHqW0dFoQwHN
(cherry picked from commit e15ddb7f9a)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The luma interpolation filters read two lines above and three lines
below a block, so rv34_mc() needs 8 * height + 5 reference lines, but it
had emulated_edge_mc() copy one more.
Fixes: data race between rv40_loop_filter() and rv34_mc() under frame threading
(cherry picked from commit 709f8fecec)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: unsynchronized cross-thread read of reference frame rows
Fixes: IFu1maPxsZFJ
Found-by: Mozilla's AI bot, reviewed and reported by Chun-Min Chang <cchang@mozilla.com>
(cherry picked from commit 9e17d6c9f1)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
vp9_decode_flush() releases s->s.frames[], s->s.refs[] and s->s.ref_frames[]
but leaves s->next_refs[] referenced. Under frame threading,
vp9_decode_update_thread_context() seeds a worker's s->s.refs[] from the source
worker's next_refs[] (not from its refs[]). The pre-flush next_refs therefore
survive a flush and can be resurrected into a worker's refs[] afterwards,
letting a subsequent inter frame pass the reference-availability check and
decode against references that no longer exist.
Unref s->next_refs[] alongside s->s.refs[] so that flushing fully drops the
decoder's reference state.
Fixes: heap out-of-bounds read and write after avcodec_flush_buffers()
Fixes: 34c2zQaUquOl
Found-by: Mozilla's bugmon automation, reviewed and reported by Chun-Min Chang <cchang@mozilla.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit a024f8c541)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
