* [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes
@ 2024-02-03 17:21 post
2024-02-05 12:45 ` Nuo Mi
0 siblings, 1 reply; 4+ messages in thread
From: post @ 2024-02-03 17:21 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Frank Plowman
From: Frank Plowman <post@frankplowman.com>
Fixes crashes when decoding illegal bitstreams found by fuzzing.
Signed-off-by: Frank Plowman <post@frankplowman.com>
---
libavcodec/vvc/vvc_ctu.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
index d166b16a19..f601c6c95e 100644
--- a/libavcodec/vvc/vvc_ctu.c
+++ b/libavcodec/vvc/vvc_ctu.c
@@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int rx, const int ry)
}
}
-static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
+static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
{
const VVCFrameContext *fc = lc->fc;
const H266RawSliceHeader *sh = lc->sc->sh.r;
@@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
c_idx == CB ? sh->sh_alf_cb_enabled_flag : sh->sh_alf_cr_enabled_flag;
if (alf_enabled_flag) {
const VVCALF *aps = fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
+ if (!aps) {
+ // Slice header refers to an APS which does not exist
+ return AVERROR_INVALIDDATA;
+ }
+
alf->ctb_flag[c_idx] = ff_vvc_alf_ctb_flag(lc, rx, ry, c_idx);
alf->alf_ctb_filter_alt_idx[c_idx - 1] = 0;
if (alf->ctb_flag[c_idx] && aps->num_chroma_filters > 1)
@@ -2247,10 +2252,16 @@ static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
alf->ctb_cc_idc[i] = 0;
if (cc_enabled[i]) {
const VVCALF *aps = fc->ps.alf_list[cc_aps_id[i]];
+ if (!aps) {
+ // Slice header refers to an APS which does not exist
+ return AVERROR_INVALIDDATA;
+ }
alf->ctb_cc_idc[i] = ff_vvc_alf_ctb_cc_idc(lc, rx, ry, i, aps->num_cc_filters[i]);
}
}
}
+
+ return 0;
}
static void deblock_params(VVCLocalContext *lc, const int rx, const int ry)
@@ -2274,7 +2285,9 @@ static int hls_coding_tree_unit(VVCLocalContext *lc,
memset(lc->parse.chroma_qp_offset, 0, sizeof(lc->parse.chroma_qp_offset));
hls_sao(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
- alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
+ ret = alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
+ if (ret < 0)
+ return ret;
deblock_params(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
if (IS_I(rsh) && sps->r->sps_qtbtt_dual_tree_intra_flag)
--
2.43.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes
2024-02-03 17:21 [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes post
@ 2024-02-05 12:45 ` Nuo Mi
2024-02-05 14:04 ` Frank Plowman
0 siblings, 1 reply; 4+ messages in thread
From: Nuo Mi @ 2024-02-05 12:45 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Frank Plowman
On Sun, Feb 4, 2024 at 1:22 AM <post@frankplowman.com> wrote:
> From: Frank Plowman <post@frankplowman.com>
>
> Fixes crashes when decoding illegal bitstreams found by fuzzing.
>
> Signed-off-by: Frank Plowman <post@frankplowman.com>
> ---
> libavcodec/vvc/vvc_ctu.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
> index d166b16a19..f601c6c95e 100644
> --- a/libavcodec/vvc/vvc_ctu.c
> +++ b/libavcodec/vvc/vvc_ctu.c
> @@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int
> rx, const int ry)
> }
> }
>
> -static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
> +static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
> {
> const VVCFrameContext *fc = lc->fc;
> const H266RawSliceHeader *sh = lc->sc->sh.r;
> @@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const
> int rx, const int ry)
> c_idx == CB ? sh->sh_alf_cb_enabled_flag :
> sh->sh_alf_cr_enabled_flag;
> if (alf_enabled_flag) {
> const VVCALF *aps =
> fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
> + if (!aps) {
> + // Slice header refers to an APS which does not exist
> + return AVERROR_INVALIDDATA;
> + }
> +
>
Thank you Frank.
Since all "if conditions" are related to slices, could we perform the check
at sh_derive()?
> alf->ctb_flag[c_idx] = ff_vvc_alf_ctb_flag(lc, rx, ry,
> c_idx);
> alf->alf_ctb_filter_alt_idx[c_idx - 1] = 0;
> if (alf->ctb_flag[c_idx] && aps->num_chroma_filters > 1)
> @@ -2247,10 +2252,16 @@ static void alf_params(VVCLocalContext *lc, const
> int rx, const int ry)
> alf->ctb_cc_idc[i] = 0;
> if (cc_enabled[i]) {
> const VVCALF *aps = fc->ps.alf_list[cc_aps_id[i]];
> + if (!aps) {
> + // Slice header refers to an APS which does not exist
> + return AVERROR_INVALIDDATA;
> + }
> alf->ctb_cc_idc[i] = ff_vvc_alf_ctb_cc_idc(lc, rx, ry, i,
> aps->num_cc_filters[i]);
> }
> }
> }
> +
> + return 0;
> }
>
> static void deblock_params(VVCLocalContext *lc, const int rx, const int
> ry)
> @@ -2274,7 +2285,9 @@ static int hls_coding_tree_unit(VVCLocalContext *lc,
> memset(lc->parse.chroma_qp_offset, 0,
> sizeof(lc->parse.chroma_qp_offset));
>
> hls_sao(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
> - alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
> sps->ctb_log2_size_y);
> + ret = alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
> sps->ctb_log2_size_y);
> + if (ret < 0)
> + return ret;
> deblock_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
> sps->ctb_log2_size_y);
>
> if (IS_I(rsh) && sps->r->sps_qtbtt_dual_tree_intra_flag)
> --
> 2.43.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes
2024-02-05 12:45 ` Nuo Mi
@ 2024-02-05 14:04 ` Frank Plowman
2024-02-05 14:31 ` Nuo Mi
0 siblings, 1 reply; 4+ messages in thread
From: Frank Plowman @ 2024-02-05 14:04 UTC (permalink / raw)
To: ffmpeg-devel
On 05/02/2024 13:45, Nuo Mi wrote:
> On Sun, Feb 4, 2024 at 1:22 AM <post@frankplowman.com> wrote:
>
>> From: Frank Plowman <post@frankplowman.com>
>>
>> Fixes crashes when decoding illegal bitstreams found by fuzzing.
>>
>> Signed-off-by: Frank Plowman <post@frankplowman.com>
>> ---
>> libavcodec/vvc/vvc_ctu.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
>> index d166b16a19..f601c6c95e 100644
>> --- a/libavcodec/vvc/vvc_ctu.c
>> +++ b/libavcodec/vvc/vvc_ctu.c
>> @@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int
>> rx, const int ry)
>> }
>> }
>>
>> -static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
>> +static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
>> {
>> const VVCFrameContext *fc = lc->fc;
>> const H266RawSliceHeader *sh = lc->sc->sh.r;
>> @@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const
>> int rx, const int ry)
>> c_idx == CB ? sh->sh_alf_cb_enabled_flag :
>> sh->sh_alf_cr_enabled_flag;
>> if (alf_enabled_flag) {
>> const VVCALF *aps =
>> fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
>> + if (!aps) {
>> + // Slice header refers to an APS which does not exist
>> + return AVERROR_INVALIDDATA;
>> + }
>> +
>>
> Thank you Frank.
> Since all "if conditions" are related to slices, could we perform the check
> at sh_derive()?
Yes, I had another look at this yesterday and came to the same
conclusion. Do you think it's better to derive pointers to the relevant
APSs or just to validate the indices into fps->aps_list?
>
>> alf->ctb_flag[c_idx] = ff_vvc_alf_ctb_flag(lc, rx, ry,
>> c_idx);
>> alf->alf_ctb_filter_alt_idx[c_idx - 1] = 0;
>> if (alf->ctb_flag[c_idx] && aps->num_chroma_filters > 1)
>> @@ -2247,10 +2252,16 @@ static void alf_params(VVCLocalContext *lc, const
>> int rx, const int ry)
>> alf->ctb_cc_idc[i] = 0;
>> if (cc_enabled[i]) {
>> const VVCALF *aps = fc->ps.alf_list[cc_aps_id[i]];
>> + if (!aps) {
>> + // Slice header refers to an APS which does not exist
>> + return AVERROR_INVALIDDATA;
>> + }
>> alf->ctb_cc_idc[i] = ff_vvc_alf_ctb_cc_idc(lc, rx, ry, i,
>> aps->num_cc_filters[i]);
>> }
>> }
>> }
>> +
>> + return 0;
>> }
>>
>> static void deblock_params(VVCLocalContext *lc, const int rx, const int
>> ry)
>> @@ -2274,7 +2285,9 @@ static int hls_coding_tree_unit(VVCLocalContext *lc,
>> memset(lc->parse.chroma_qp_offset, 0,
>> sizeof(lc->parse.chroma_qp_offset));
>>
>> hls_sao(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
>> - alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>> + ret = alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>> + if (ret < 0)
>> + return ret;
>> deblock_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>>
>> if (IS_I(rsh) && sps->r->sps_qtbtt_dual_tree_intra_flag)
>> --
>> 2.43.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes
2024-02-05 14:04 ` Frank Plowman
@ 2024-02-05 14:31 ` Nuo Mi
0 siblings, 0 replies; 4+ messages in thread
From: Nuo Mi @ 2024-02-05 14:31 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, Feb 5, 2024 at 10:04 PM Frank Plowman <post@frankplowman.com> wrote:
> On 05/02/2024 13:45, Nuo Mi wrote:
> > On Sun, Feb 4, 2024 at 1:22 AM <post@frankplowman.com> wrote:
> >
> >> From: Frank Plowman <post@frankplowman.com>
> >>
> >> Fixes crashes when decoding illegal bitstreams found by fuzzing.
> >>
> >> Signed-off-by: Frank Plowman <post@frankplowman.com>
> >> ---
> >> libavcodec/vvc/vvc_ctu.c | 17 +++++++++++++++--
> >> 1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
> >> index d166b16a19..f601c6c95e 100644
> >> --- a/libavcodec/vvc/vvc_ctu.c
> >> +++ b/libavcodec/vvc/vvc_ctu.c
> >> @@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int
> >> rx, const int ry)
> >> }
> >> }
> >>
> >> -static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
> >> +static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
> >> {
> >> const VVCFrameContext *fc = lc->fc;
> >> const H266RawSliceHeader *sh = lc->sc->sh.r;
> >> @@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const
> >> int rx, const int ry)
> >> c_idx == CB ? sh->sh_alf_cb_enabled_flag :
> >> sh->sh_alf_cr_enabled_flag;
> >> if (alf_enabled_flag) {
> >> const VVCALF *aps =
> >> fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
> >> + if (!aps) {
> >> + // Slice header refers to an APS which does not
> exist
> >> + return AVERROR_INVALIDDATA;
> >> + }
> >> +
> >>
> > Thank you Frank.
> > Since all "if conditions" are related to slices, could we perform the
> check
> > at sh_derive()?
>
> Yes, I had another look at this yesterday and came to the same
> conclusion. Do you think it's better to derive pointers to the relevant
> APSs or just to validate the indices into fps->aps_list?
>
> Check the fps->aps_list[i] should be enough.
Make sure you run all the conformance tests, only conformance tests know
what change is right :)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-05 14:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-03 17:21 [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes post
2024-02-05 12:45 ` Nuo Mi
2024-02-05 14:04 ` Frank Plowman
2024-02-05 14:31 ` Nuo Mi
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git