| Message ID | TYSPR06MB64336B18D7F446B705AF9254AAFAA@TYSPR06MB6433.apcprd06.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 9ef20920ab82c46de095499deec2777b48a19370 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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 [...]
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;
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". >
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;
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(+)