| Message ID | 20230802093524.1136658-1-kobrineli@ispras.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libswresample: Prevent out of bounds. | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| yinshiyou/make_loongarch64 | warning | New warnings during build |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_x86 | warning | New warnings during build |
kobrineli: > From: Eli Kobrin <kobrineli@ispras.ru> > > We've been fuzzing torchvision with [sydr-fuzz](https://github.com/ispras/oss-sydr-fuzz) > and found out of bounds error in ffmpeg project at audioconvert.c:51. > To prevent error we need to insert corresponding check. > > Signed-off-by: Eli Kobrin <kobrineli@ispras.ru> > --- > libswresample/audioconvert.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libswresample/audioconvert.c b/libswresample/audioconvert.c > index 1d75ba1495..701f4808a0 100644 > --- a/libswresample/audioconvert.c > +++ b/libswresample/audioconvert.c > @@ -148,7 +148,12 @@ AudioConvert *swri_audio_convert_alloc(enum AVSampleFormat out_fmt, > int flags) > { > AudioConvert *ctx; > - conv_func_type *f = fmt_pair_to_conv_functions[av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB*av_get_packed_sample_fmt(in_fmt)]; > + > + size_t idx = av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB * av_get_packed_sample_fmt(in_fmt); > + if (idx >= AV_SAMPLE_FMT_NB * AV_SAMPLE_FMT_NB) > + return NULL; > + > + conv_func_type *f = fmt_pair_to_conv_functions[idx]; > > if (!f) > return NULL; Something seems to be using an invalid sample format (either out_fmt or in_fmt). You should investigate where this comes from. (Given that this is a public function, we should probably validate user input; and maybe stop using AV_SAMPLE_FMT_NB altogether.) - Andreas
Invalid out or int fmts are got from the user input, which was discovered through fuzzing. Don't know where to add check at the time of SwrContext creating, but I think this change is redundant to at least prevent dangerous out of bounds access, which set the pointer to illegal address. On 2023-08-02 13:51, Andreas Rheinhardt wrote: > kobrineli: >> From: Eli Kobrin <kobrineli@ispras.ru> >> >> We've been fuzzing torchvision with >> [sydr-fuzz](https://github.com/ispras/oss-sydr-fuzz) >> and found out of bounds error in ffmpeg project at audioconvert.c:51. >> To prevent error we need to insert corresponding check. >> >> Signed-off-by: Eli Kobrin <kobrineli@ispras.ru> >> --- >> libswresample/audioconvert.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/libswresample/audioconvert.c >> b/libswresample/audioconvert.c >> index 1d75ba1495..701f4808a0 100644 >> --- a/libswresample/audioconvert.c >> +++ b/libswresample/audioconvert.c >> @@ -148,7 +148,12 @@ AudioConvert *swri_audio_convert_alloc(enum >> AVSampleFormat out_fmt, >> int flags) >> { >> AudioConvert *ctx; >> - conv_func_type *f = >> fmt_pair_to_conv_functions[av_get_packed_sample_fmt(out_fmt) + >> AV_SAMPLE_FMT_NB*av_get_packed_sample_fmt(in_fmt)]; >> + >> + size_t idx = av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB >> * av_get_packed_sample_fmt(in_fmt); >> + if (idx >= AV_SAMPLE_FMT_NB * AV_SAMPLE_FMT_NB) >> + return NULL; >> + >> + conv_func_type *f = fmt_pair_to_conv_functions[idx]; >> >> if (!f) >> return NULL; > > Something seems to be using an invalid sample format (either out_fmt or > in_fmt). You should investigate where this comes from. > (Given that this is a public function, we should probably validate user > input; and maybe stop using AV_SAMPLE_FMT_NB altogether.) > > - 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".
I've found out that `in_fmt` is equal to -1 at the place of error, so we just need to insert check at the beginning of `swr_init` function to check fmts positivity. On 2023-08-02 13:51, Andreas Rheinhardt wrote: > kobrineli: > >> From: Eli Kobrin <kobrineli@ispras.ru> >> >> We've been fuzzing torchvision with >> [sydr-fuzz](https://github.com/ispras/oss-sydr-fuzz) >> and found out of bounds error in ffmpeg project at audioconvert.c:51. >> To prevent error we need to insert corresponding check. >> >> Signed-off-by: Eli Kobrin <kobrineli@ispras.ru> >> --- >> libswresample/audioconvert.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/libswresample/audioconvert.c >> b/libswresample/audioconvert.c >> index 1d75ba1495..701f4808a0 100644 >> --- a/libswresample/audioconvert.c >> +++ b/libswresample/audioconvert.c >> @@ -148,7 +148,12 @@ AudioConvert *swri_audio_convert_alloc(enum >> AVSampleFormat out_fmt, >> int flags) >> { >> AudioConvert *ctx; >> - conv_func_type *f = >> fmt_pair_to_conv_functions[av_get_packed_sample_fmt(out_fmt) + >> AV_SAMPLE_FMT_NB*av_get_packed_sample_fmt(in_fmt)]; >> + >> + size_t idx = av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB >> * av_get_packed_sample_fmt(in_fmt); >> + if (idx >= AV_SAMPLE_FMT_NB * AV_SAMPLE_FMT_NB) >> + return NULL; >> + >> + conv_func_type *f = fmt_pair_to_conv_functions[idx]; >> >> if (!f) >> return NULL; > > Something seems to be using an invalid sample format (either out_fmt or > in_fmt). You should investigate where this comes from. > (Given that this is a public function, we should probably validate user > input; and maybe stop using AV_SAMPLE_FMT_NB altogether.) > > - 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".
Resubmitted the patch (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230802113106.1138555-1-kobrineli@ispras.ru/). Didn't understand how to fix the existing patch. On 2023-08-02 13:51, Andreas Rheinhardt wrote: > kobrineli: >> From: Eli Kobrin <kobrineli@ispras.ru> >> >> We've been fuzzing torchvision with >> [sydr-fuzz](https://github.com/ispras/oss-sydr-fuzz) >> and found out of bounds error in ffmpeg project at audioconvert.c:51. >> To prevent error we need to insert corresponding check. >> >> Signed-off-by: Eli Kobrin <kobrineli@ispras.ru> >> --- >> libswresample/audioconvert.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/libswresample/audioconvert.c >> b/libswresample/audioconvert.c >> index 1d75ba1495..701f4808a0 100644 >> --- a/libswresample/audioconvert.c >> +++ b/libswresample/audioconvert.c >> @@ -148,7 +148,12 @@ AudioConvert *swri_audio_convert_alloc(enum >> AVSampleFormat out_fmt, >> int flags) >> { >> AudioConvert *ctx; >> - conv_func_type *f = >> fmt_pair_to_conv_functions[av_get_packed_sample_fmt(out_fmt) + >> AV_SAMPLE_FMT_NB*av_get_packed_sample_fmt(in_fmt)]; >> + >> + size_t idx = av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB >> * av_get_packed_sample_fmt(in_fmt); >> + if (idx >= AV_SAMPLE_FMT_NB * AV_SAMPLE_FMT_NB) >> + return NULL; >> + >> + conv_func_type *f = fmt_pair_to_conv_functions[idx]; >> >> if (!f) >> return NULL; > > Something seems to be using an invalid sample format (either out_fmt or > in_fmt). You should investigate where this comes from. > (Given that this is a public function, we should probably validate user > input; and maybe stop using AV_SAMPLE_FMT_NB altogether.) > > - 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".
diff --git a/libswresample/audioconvert.c b/libswresample/audioconvert.c index 1d75ba1495..701f4808a0 100644 --- a/libswresample/audioconvert.c +++ b/libswresample/audioconvert.c @@ -148,7 +148,12 @@ AudioConvert *swri_audio_convert_alloc(enum AVSampleFormat out_fmt, int flags) { AudioConvert *ctx; - conv_func_type *f = fmt_pair_to_conv_functions[av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB*av_get_packed_sample_fmt(in_fmt)]; + + size_t idx = av_get_packed_sample_fmt(out_fmt) + AV_SAMPLE_FMT_NB * av_get_packed_sample_fmt(in_fmt); + if (idx >= AV_SAMPLE_FMT_NB * AV_SAMPLE_FMT_NB) + return NULL; + + conv_func_type *f = fmt_pair_to_conv_functions[idx]; if (!f) return NULL;