[FFmpeg-devel] [PATCH] avcodec/rv60dec: add upper bound check for qp

[FFmpeg-devel] [PATCH] avcodec/rv60dec: add upper bound check for qp

[Reaxx via ffmpeg-devel]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

This patch fixes an out-of-bounds read in the RV60 decoder where qp can
reach 65, exceeding the rv60_qp_to_idx[64] array bounds. The previous fix
(61cbcaf93f) only covered intra frames. This adds validation at the source
for all frame types.

[-- Attachment #2: 0001-avcodec-rv60dec-add-upper-bound-check-for-qp.patch --]
[-- Type: application/octet-stream, Size: 1531 bytes --]

[-- Attachment #3: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avcodec/rv60dec: add upper bound check for qp

[Michael Niedermayer via ffmpeg-devel]

[-- Attachment #1.1: Type: text/plain, Size: 1928 bytes --]

Hi Reaxx

On Fri, Nov 07, 2025 at 06:20:38PM +0100, Reaxx via ffmpeg-devel wrote:
> This patch fixes an out-of-bounds read in the RV60 decoder where qp can
> reach 65, exceeding the rv60_qp_to_idx[64] array bounds. The previous fix
> (61cbcaf93f) only covered intra frames. This adds validation at the source
> for all frame types.

>  rv60dec.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 57b6f4d726a8d8959a14807b0fa1fb162ce7bd74  /var0001-avcodec-rv60dec-add-upper-bound-check-for-qp.patch
> From c7a4ef1c2d235e73be849028c145949eac6ae9ef Mon Sep 17 00:00:00 2001
> From: oblivionsage <cookieandcream560@gmail.com>
> Date: Fri, 7 Nov 2025 18:08:14 +0100
> Subject: [PATCH] avcodec/rv60dec: add upper bound check for qp
> 
> The quantization parameter (qp) can exceed 63 when the base value
> from frame header (0-63) is combined with the offset from slice data
> (up to +2), resulting in qp=65. This causes out-of-bounds access to
> the rv60_qp_to_idx[64] array in decode_cbp8(), decode_cbp16(), and
> get_c4x4_set().
> 

> Previous fix in commit 61cbcaf93f3b2e10124f4c63ce7cd8dad6505fb2 added validation only for intra
> frames at a later stage. This patch adds validation at the source
> in decode_slice() to prevent invalid qp values for all frame types.

This is not correct, the current code is not just checking qp in the
intra case


> 
> Fixes: Out-of-bounds read reported by OSS-Fuzz (clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RV60_fuzzer-5160167345291264)

This is incorrect, this testcase does not trigger the issue fixed in this patch

There is in fact no testcase for the issue this fixes to the best of
my knowledge.

i will apply this with a corrected commit message

thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avcodec/rv60dec: add upper bound check for qp

[Reaxx via ffmpeg-devel]

Thanks for reviewing and applying the patch. Apologies for the incorrect
commit message , I appreciate you correcting it. Thanks again

On Fri, 7 Nov 2025 at 22:14, Michael Niedermayer via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:

> Hi Reaxx
>
> On Fri, Nov 07, 2025 at 06:20:38PM +0100, Reaxx via ffmpeg-devel wrote:
> > This patch fixes an out-of-bounds read in the RV60 decoder where qp can
> > reach 65, exceeding the rv60_qp_to_idx[64] array bounds. The previous fix
> > (61cbcaf93f) only covered intra frames. This adds validation at the
> source
> > for all frame types.
>
> >  rv60dec.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 57b6f4d726a8d8959a14807b0fa1fb162ce7bd74
> /var0001-avcodec-rv60dec-add-upper-bound-check-for-qp.patch
> > From c7a4ef1c2d235e73be849028c145949eac6ae9ef Mon Sep 17 00:00:00 2001
> > From: oblivionsage <cookieandcream560@gmail.com>
> > Date: Fri, 7 Nov 2025 18:08:14 +0100
> > Subject: [PATCH] avcodec/rv60dec: add upper bound check for qp
> >
> > The quantization parameter (qp) can exceed 63 when the base value
> > from frame header (0-63) is combined with the offset from slice data
> > (up to +2), resulting in qp=65. This causes out-of-bounds access to
> > the rv60_qp_to_idx[64] array in decode_cbp8(), decode_cbp16(), and
> > get_c4x4_set().
> >
>
> > Previous fix in commit 61cbcaf93f3b2e10124f4c63ce7cd8dad6505fb2 added
> validation only for intra
> > frames at a later stage. This patch adds validation at the source
> > in decode_slice() to prevent invalid qp values for all frame types.
>
> This is not correct, the current code is not just checking qp in the
> intra case
>
>
> >
> > Fixes: Out-of-bounds read reported by OSS-Fuzz
> (clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RV60_fuzzer-5160167345291264)
>
> This is incorrect, this testcase does not trigger the issue fixed in this
> patch
>
> There is in fact no testcase for the issue this fixes to the best of
> my knowledge.
>
> i will apply this with a corrected commit message
>
> thx
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
> _______________________________________________
> ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
> To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
>
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avcodec/rv60dec: add upper bound check for qp

[Michael Niedermayer via ffmpeg-devel]

[-- Attachment #1.1: Type: text/plain, Size: 2067 bytes --]

Hi

On Fri, Nov 07, 2025 at 10:19:27PM +0100, Reaxx via ffmpeg-devel wrote:
> Thanks for reviewing and applying the patch. Apologies for the incorrect
> commit message , I appreciate you correcting it. Thanks again

actually, can you check if my commit message is correct before i apply it ?

commit 3adc44bce403bf0841d7c1a933f2635037a762f8 (HEAD -> master)
Author: oblivionsage <cookieandcream560@gmail.com>
Date:   Fri Nov 7 18:08:14 2025 +0100

    avcodec/rv60dec: add upper bound check for qp

    The quantization parameter (qp) can exceed 63 when the base value
    from frame header (0-63) is combined with the offset from slice data
    (up to +2), resulting in qp=65. This causes out-of-bounds access to
    the rv60_qp_to_idx[64] array in decode_cbp8(), decode_cbp16(), and
    get_c4x4_set().

    Fixes: Out-of-bounds read
    Signed-off-by: oblivionsage <cookieandcream560@gmail.com>

    No testsample is available

    This is related to 61cbcaf93f3b2e10124f4c63ce7cd8dad6505fb2 and clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RV60_fuzzer-5160167345291264
    which fixed rv60_qp_to_idx[qp + 32] out of array access
    These 2 checks are not redundant and neither covers the cases of the other

    Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

diff --git a/libavcodec/rv60dec.c b/libavcodec/rv60dec.c
index 33728e33a06..b7b4f46512e 100644
--- a/libavcodec/rv60dec.c
+++ b/libavcodec/rv60dec.c
@@ -2265,7 +2265,7 @@ static int decode_slice(AVCodecContext *avctx, void *tdata, int cu_y, int thread
             ff_thread_progress_await(&s->progress[cu_y - 1], cu_x + 2);

         qp = s->qp + read_qp_offset(&gb, s->qp_off_type);
-        if (qp < 0) {
+        if (qp < 0 || qp >= 64) {
             ret = AVERROR_INVALIDDATA;
             break;


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avcodec/rv60dec: add upper bound check for qp

[Reaxx via ffmpeg-devel]

Hi Michael,

Yes, the commit message LGTM , Thank you for the detailed explanation and
for taking the time to review this thoroughly. I appreciate your patience.

On Fri, 7 Nov 2025 at 22:39, Michael Niedermayer via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:

> Hi
>
> On Fri, Nov 07, 2025 at 10:19:27PM +0100, Reaxx via ffmpeg-devel wrote:
> > Thanks for reviewing and applying the patch. Apologies for the incorrect
> > commit message , I appreciate you correcting it. Thanks again
>
> actually, can you check if my commit message is correct before i apply it ?
>
> commit 3adc44bce403bf0841d7c1a933f2635037a762f8 (HEAD -> master)
> Author: oblivionsage <cookieandcream560@gmail.com>
> Date:   Fri Nov 7 18:08:14 2025 +0100
>
>     avcodec/rv60dec: add upper bound check for qp
>
>     The quantization parameter (qp) can exceed 63 when the base value
>     from frame header (0-63) is combined with the offset from slice data
>     (up to +2), resulting in qp=65. This causes out-of-bounds access to
>     the rv60_qp_to_idx[64] array in decode_cbp8(), decode_cbp16(), and
>     get_c4x4_set().
>
>     Fixes: Out-of-bounds read
>     Signed-off-by: oblivionsage <cookieandcream560@gmail.com>
>
>     No testsample is available
>
>     This is related to 61cbcaf93f3b2e10124f4c63ce7cd8dad6505fb2 and
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RV60_fuzzer-5160167345291264
>     which fixed rv60_qp_to_idx[qp + 32] out of array access
>     These 2 checks are not redundant and neither covers the cases of the
> other
>
>     Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>
> diff --git a/libavcodec/rv60dec.c b/libavcodec/rv60dec.c
> index 33728e33a06..b7b4f46512e 100644
> --- a/libavcodec/rv60dec.c
> +++ b/libavcodec/rv60dec.c
> @@ -2265,7 +2265,7 @@ static int decode_slice(AVCodecContext *avctx, void
> *tdata, int cu_y, int thread
>              ff_thread_progress_await(&s->progress[cu_y - 1], cu_x + 2);
>
>          qp = s->qp + read_qp_offset(&gb, s->qp_off_type);
> -        if (qp < 0) {
> +        if (qp < 0 || qp >= 64) {
>              ret = AVERROR_INVALIDDATA;
>              break;
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
> _______________________________________________
> ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
> To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
>
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avcodec/rv60dec: add upper bound check for qp

[Michael Niedermayer via ffmpeg-devel]

[-- Attachment #1.1: Type: text/plain, Size: 450 bytes --]

Hi Reaxx

On Fri, Nov 07, 2025 at 10:49:05PM +0100, Reaxx via ffmpeg-devel wrote:
> Hi Michael,
> 
> Yes, the commit message LGTM , Thank you for the detailed explanation and
> for taking the time to review this thoroughly. I appreciate your patience.

patch applied

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What is money laundering? Its paying someone and not telling the government.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org