* [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len
@ 2024-02-09 11:16 Nuo Mi
2024-02-09 11:41 ` Andreas Rheinhardt
0 siblings, 1 reply; 5+ messages in thread
From: Nuo Mi @ 2024-02-09 11:16 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Michael Niedermayer, Nuo Mi, Andreas Rheinhardt
Fixes: fuzzer timeout
Fixes: 65253/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_MP4TOANNEXB_fuzzer-4972412487467008
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/bsf/vvc_mp4toannexb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/bsf/vvc_mp4toannexb.c b/libavcodec/bsf/vvc_mp4toannexb.c
index 25c3726918..36bdae8f49 100644
--- a/libavcodec/bsf/vvc_mp4toannexb.c
+++ b/libavcodec/bsf/vvc_mp4toannexb.c
@@ -155,10 +155,11 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx)
}
for (j = 0; j < cnt; j++) {
- int nalu_len = bytestream2_get_be16(&gb);
+ const int nalu_len = bytestream2_get_be16(&gb);
- if (4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
- SIZE_MAX - new_extradata_size) {
+ if (!nalu_len ||
+ nalu_len > bytestream2_get_bytes_left(&gb) ||
+ 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) {
ret = AVERROR_INVALIDDATA;
goto fail;
}
--
2.25.1
_______________________________________________
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] 5+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len
2024-02-09 11:16 [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len Nuo Mi
@ 2024-02-09 11:41 ` Andreas Rheinhardt
2024-02-10 10:18 ` Nuo Mi
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Rheinhardt @ 2024-02-09 11:41 UTC (permalink / raw)
To: ffmpeg-devel
Nuo Mi:
> Fixes: fuzzer timeout
> Fixes: 65253/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_MP4TOANNEXB_fuzzer-4972412487467008
>
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavcodec/bsf/vvc_mp4toannexb.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/bsf/vvc_mp4toannexb.c b/libavcodec/bsf/vvc_mp4toannexb.c
> index 25c3726918..36bdae8f49 100644
> --- a/libavcodec/bsf/vvc_mp4toannexb.c
> +++ b/libavcodec/bsf/vvc_mp4toannexb.c
> @@ -155,10 +155,11 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx)
> }
>
> for (j = 0; j < cnt; j++) {
> - int nalu_len = bytestream2_get_be16(&gb);
> + const int nalu_len = bytestream2_get_be16(&gb);
>
> - if (4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
> - SIZE_MAX - new_extradata_size) {
> + if (!nalu_len ||
> + nalu_len > bytestream2_get_bytes_left(&gb) ||
> + 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX - new_extradata_size) {
> ret = AVERROR_INVALIDDATA;
> goto fail;
> }
What about growing the packet?
- Andreas
_______________________________________________
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] 5+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len
2024-02-09 11:41 ` Andreas Rheinhardt
@ 2024-02-10 10:18 ` Nuo Mi
2024-02-10 10:59 ` Andreas Rheinhardt
0 siblings, 1 reply; 5+ messages in thread
From: Nuo Mi @ 2024-02-10 10:18 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, Feb 9, 2024 at 7:40 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:
> Nuo Mi:
> > Fixes: fuzzer timeout
> > Fixes:
> 65253/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_MP4TOANNEXB_fuzzer-4972412487467008
> >
> > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> > ---
> > libavcodec/bsf/vvc_mp4toannexb.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/bsf/vvc_mp4toannexb.c
> b/libavcodec/bsf/vvc_mp4toannexb.c
> > index 25c3726918..36bdae8f49 100644
> > --- a/libavcodec/bsf/vvc_mp4toannexb.c
> > +++ b/libavcodec/bsf/vvc_mp4toannexb.c
> > @@ -155,10 +155,11 @@ static int vvc_extradata_to_annexb(AVBSFContext
> *ctx)
> > }
> >
> > for (j = 0; j < cnt; j++) {
> > - int nalu_len = bytestream2_get_be16(&gb);
> > + const int nalu_len = bytestream2_get_be16(&gb);
> >
> > - if (4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
> > - SIZE_MAX - new_extradata_size) {
> > + if (!nalu_len ||
> > + nalu_len > bytestream2_get_bytes_left(&gb) ||
> > + 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX
> - new_extradata_size) {
> > ret = AVERROR_INVALIDDATA;
> > goto fail;
> > }
>
> What about growing the packet?
>
Hi Andreas,
Do you mean growing the packet only once for all nalus?
However, this would change the original behavior and result in more
duplicate code between the HEVC and VVC implementations.
I can do it, but I'll refactor duplications to h2656_mp4toannexb.c first.
Do you think that's okay?
Thank you
> - Andreas
>
> _______________________________________________
> 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] 5+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len
2024-02-10 10:18 ` Nuo Mi
@ 2024-02-10 10:59 ` Andreas Rheinhardt
2024-02-11 14:57 ` Nuo Mi
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Rheinhardt @ 2024-02-10 10:59 UTC (permalink / raw)
To: ffmpeg-devel
Nuo Mi:
> On Fri, Feb 9, 2024 at 7:40 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
>
>> Nuo Mi:
>>> Fixes: fuzzer timeout
>>> Fixes:
>> 65253/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_MP4TOANNEXB_fuzzer-4972412487467008
>>>
>>> Found-by: continuous fuzzing process
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>> libavcodec/bsf/vvc_mp4toannexb.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/bsf/vvc_mp4toannexb.c
>> b/libavcodec/bsf/vvc_mp4toannexb.c
>>> index 25c3726918..36bdae8f49 100644
>>> --- a/libavcodec/bsf/vvc_mp4toannexb.c
>>> +++ b/libavcodec/bsf/vvc_mp4toannexb.c
>>> @@ -155,10 +155,11 @@ static int vvc_extradata_to_annexb(AVBSFContext
>> *ctx)
>>> }
>>>
>>> for (j = 0; j < cnt; j++) {
>>> - int nalu_len = bytestream2_get_be16(&gb);
>>> + const int nalu_len = bytestream2_get_be16(&gb);
>>>
>>> - if (4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
>>> - SIZE_MAX - new_extradata_size) {
>>> + if (!nalu_len ||
>>> + nalu_len > bytestream2_get_bytes_left(&gb) ||
>>> + 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX
>> - new_extradata_size) {
>>> ret = AVERROR_INVALIDDATA;
>>> goto fail;
>>> }
>>
>> What about growing the packet?
>>
> Hi Andreas,
> Do you mean growing the packet only once for all nalus?
> However, this would change the original behavior and result in more
> duplicate code between the HEVC and VVC implementations.
> I can do it, but I'll refactor duplications to h2656_mp4toannexb.c first.
> Do you think that's okay?
> Thank you
>
I meant a simple check before growing the packet to ensure that there is
enough input left as the header claims there to be. Your original patch
added such a check, so I presumed that there is none. But there is one
(both here and in hevc_mp4toannexb). So my above point is moot.
- Andreas
_______________________________________________
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] 5+ messages in thread* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len
2024-02-10 10:59 ` Andreas Rheinhardt
@ 2024-02-11 14:57 ` Nuo Mi
0 siblings, 0 replies; 5+ messages in thread
From: Nuo Mi @ 2024-02-11 14:57 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sat, Feb 10, 2024 at 6:57 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:
> Nuo Mi:
> > On Fri, Feb 9, 2024 at 7:40 PM Andreas Rheinhardt <
> > andreas.rheinhardt@outlook.com> wrote:
> >
> >> Nuo Mi:
> >>> Fixes: fuzzer timeout
> >>> Fixes:
> >>
> 65253/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_MP4TOANNEXB_fuzzer-4972412487467008
> >>>
> >>> Found-by: continuous fuzzing process
> >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> >>> ---
> >>> libavcodec/bsf/vvc_mp4toannexb.c | 7 ++++---
> >>> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/libavcodec/bsf/vvc_mp4toannexb.c
> >> b/libavcodec/bsf/vvc_mp4toannexb.c
> >>> index 25c3726918..36bdae8f49 100644
> >>> --- a/libavcodec/bsf/vvc_mp4toannexb.c
> >>> +++ b/libavcodec/bsf/vvc_mp4toannexb.c
> >>> @@ -155,10 +155,11 @@ static int vvc_extradata_to_annexb(AVBSFContext
> >> *ctx)
> >>> }
> >>>
> >>> for (j = 0; j < cnt; j++) {
> >>> - int nalu_len = bytestream2_get_be16(&gb);
> >>> + const int nalu_len = bytestream2_get_be16(&gb);
> >>>
> >>> - if (4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
> >>> - SIZE_MAX - new_extradata_size) {
> >>> + if (!nalu_len ||
> >>> + nalu_len > bytestream2_get_bytes_left(&gb) ||
> >>> + 4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len > SIZE_MAX
> >> - new_extradata_size) {
> >>> ret = AVERROR_INVALIDDATA;
> >>> goto fail;
> >>> }
> >>
> >> What about growing the packet?
> >>
> > Hi Andreas,
> > Do you mean growing the packet only once for all nalus?
> > However, this would change the original behavior and result in more
> > duplicate code between the HEVC and VVC implementations.
> > I can do it, but I'll refactor duplications to h2656_mp4toannexb.c first.
> > Do you think that's okay?
> > Thank you
> >
>
> I meant a simple check before growing the packet to ensure that there is
> enough input left as the header claims there to be. Your original patch
> added such a check, so I presumed that there is none. But there is one
> (both here and in hevc_mp4toannexb). So my above point is moot.
Thank you for the explanation. Your suggestion for the original patch is
pretty good. Thank you
Applied.
>
> - Andreas
>
> _______________________________________________
> 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] 5+ messages in thread
end of thread, other threads:[~2024-02-11 14:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-09 11:16 [FFmpeg-devel] [PATCH v2 1/2] avcodec/vvc_mp4toannexb: check bytes left for nalu_len Nuo Mi
2024-02-09 11:41 ` Andreas Rheinhardt
2024-02-10 10:18 ` Nuo Mi
2024-02-10 10:59 ` Andreas Rheinhardt
2024-02-11 14:57 ` 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