[FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[Nuo Mi]

from the specification:
For each OLS, there shall be at least one layer that is an output layer. In other words, for any value of i in the range of 0
to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ] shall be greater than or equal to 1

Fixes: index 257 out of bounds for type 'uint8_t [257]'
Fixes: 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/cbs_h266_syntax_template.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c
index f0d428a478..2077aa63bf 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -891,6 +891,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw,
                     }
                 }
             }
+            if (!num_output_layers_in_ols[i])
+                return AVERROR_INVALIDDATA;
         }
         for (i = 1; i < total_num_olss; i++) {
             int num_layers_in_ols = 0;
-- 
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".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[Nuo Mi]

On Tue, Sep 19, 2023 at 11:26 PM Nuo Mi <nuomi2021@gmail.com> wrote:

> from the specification:
> For each OLS, there shall be at least one layer that is an output layer.
> In other words, for any value of i in the range of 0
> to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ]
> shall be greater than or equal to 1
>
> Fixes: index 257 out of bounds for type 'uint8_t [257]'
> Fixes:
> 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024
>
replaces
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230905020358.32527-2-michael@niedermayer.cc/


>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/cbs_h266_syntax_template.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> index f0d428a478..2077aa63bf 100644
> --- a/libavcodec/cbs_h266_syntax_template.c
> +++ b/libavcodec/cbs_h266_syntax_template.c
> @@ -891,6 +891,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
> RWContext *rw,
>                      }
>                  }
>              }
> +            if (!num_output_layers_in_ols[i])
> +                return AVERROR_INVALIDDATA;
>          }
>          for (i = 1; i < total_num_olss; i++) {
>              int num_layers_in_ols = 0;
> --
> 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".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[Michael Niedermayer]

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

On Tue, Sep 19, 2023 at 11:28:20PM +0800, Nuo Mi wrote:
> On Tue, Sep 19, 2023 at 11:26 PM Nuo Mi <nuomi2021@gmail.com> wrote:
> 
> > from the specification:
> > For each OLS, there shall be at least one layer that is an output layer.
> > In other words, for any value of i in the range of 0
> > to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ]
> > shall be greater than or equal to 1
> >
> > Fixes: index 257 out of bounds for type 'uint8_t [257]'
> > Fixes:
> > 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024
> >
> replaces
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230905020358.32527-2-michael@niedermayer.cc/

will apply your patch

thanks

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

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.

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

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

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[James Almer]

On 9/19/2023 12:25 PM, Nuo Mi wrote:
> from the specification:
> For each OLS, there shall be at least one layer that is an output layer. In other words, for any value of i in the range of 0
> to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ] shall be greater than or equal to 1
> 
> Fixes: index 257 out of bounds for type 'uint8_t [257]'
> Fixes: 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/cbs_h266_syntax_template.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c
> index f0d428a478..2077aa63bf 100644
> --- a/libavcodec/cbs_h266_syntax_template.c
> +++ b/libavcodec/cbs_h266_syntax_template.c
> @@ -891,6 +891,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw,
>                       }
>                   }
>               }
> +            if (!num_output_layers_in_ols[i])

The only way for num_output_layers_in_ols[i] to be less than 1 is if 
none of the checks above succeeded, and since the array is stored on 
stack uninitialized, this to me looks like a potential use of 
uninitialized value.

> +                return AVERROR_INVALIDDATA;
>           }
>           for (i = 1; i < total_num_olss; i++) {
>               int num_layers_in_ols = 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".

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[Nuo Mi]

On Fri, Sep 22, 2023 at 12:04 AM James Almer <jamrial@gmail.com> wrote:

> On 9/19/2023 12:25 PM, Nuo Mi wrote:
> > from the specification:
> > For each OLS, there shall be at least one layer that is an output layer.
> In other words, for any value of i in the range of 0
> > to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ]
> shall be greater than or equal to 1
> >
> > Fixes: index 257 out of bounds for type 'uint8_t [257]'
> > Fixes:
> 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024
> >
> > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/cbs_h266_syntax_template.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> > index f0d428a478..2077aa63bf 100644
> > --- a/libavcodec/cbs_h266_syntax_template.c
> > +++ b/libavcodec/cbs_h266_syntax_template.c
> > @@ -891,6 +891,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
> RWContext *rw,
> >                       }
> >                   }
> >               }
> > +            if (!num_output_layers_in_ols[i])
>
> The only way for num_output_layers_in_ols[i] to be less than 1 is if
> none of the checks above succeeded, and since the array is stored on
> stack uninitialized, this to me looks like a potential use of
> uninitialized value.
>
Hi James,
Thank you for the review.
No. The issue triggered by the following condition.
1. current->vps_ols_mode_idc == 2
2. all current->vps_ols_output_layer_flag[i][k] is zero.
num_output_layers_in_ols[i] will be set to 0 in this case.

Actually,  num_output_layers_in_ols[i] will be set to a right value if
current->vps_ols_mode_idc equals 0, 1, 2, 4.
If current->vps_ols_mode_idc equals 3, it may have an unchecked read.
It's mainly because 3 is reserved by specification for future use, the
specification said we should ignore it.

>
> > +                return AVERROR_INVALIDDATA;
> >           }
> >           for (i = 1; i < total_num_olss; i++) {
> >               int num_layers_in_ols = 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".