[FFmpeg-devel] [PATCH 1/2] avcodec/hevc_ps: Check log2_sao_offset_scale_*

[wm4]

On Wed, 24 Jan 2018 23:42:44 -0300
James Almer <jamrial at gmail.com> wrote:

> On 1/24/2018 11:03 PM, Michael Niedermayer wrote:
> > On Wed, Jan 24, 2018 at 12:47:18AM -0300, James Almer wrote:  
> >> On 1/24/2018 12:34 AM, Michael Niedermayer wrote:  
> >>> Fixes: 4868/clusterfuzz-testcase-minimized-6236542906400768
> >>> Fixes: runtime error: shift exponent 126 is too large for 32-bit type 'int'
> >>>
> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>> ---
> >>>  libavcodec/hevc_ps.c | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> >>> index 4787312cfa..746c96b17e 100644
> >>> --- a/libavcodec/hevc_ps.c
> >>> +++ b/libavcodec/hevc_ps.c
> >>> @@ -1324,6 +1324,17 @@ static int pps_range_extensions(GetBitContext *gb, AVCodecContext *avctx,
> >>>      pps->log2_sao_offset_scale_luma = get_ue_golomb_long(gb);
> >>>      pps->log2_sao_offset_scale_chroma = get_ue_golomb_long(gb);
> >>>  
> >>> +    if (   pps->log2_sao_offset_scale_luma   > FFMAX(sps->bit_depth        - 10, 0)
> >>> +        || pps->log2_sao_offset_scale_chroma > FFMAX(sps->bit_depth_chroma - 10, 0)
> >>> +    ) {
> >>> +        av_log(avctx, AV_LOG_ERROR,
> >>> +                "log2 sao offset scales (%d %d) are invalid\n",
> >>> +               pps->log2_sao_offset_scale_luma,
> >>> +               pps->log2_sao_offset_scale_chroma
> >>> +              );
> >>> +        return AVERROR_INVALIDDATA;  
> >>
> >> Wouldn't it be better to just port the h264 and hevc decoder to use the
> >> cbs API at this point? It correctly does a range check for every
> >> sps/vps/pps/slice value already.
> >>
> >> Otherwise you'll be adding a lot of range checks as oss-fuzz finds an
> >> ubsan testcase for them.  
> > 
> > cbs is not available in the releases
> > we need to fix issues in the releases
> > 
> > so i dont think cbs can help here  
> 
> For release branches yes, no way around it, patches like this are
> needed. But for future releases it will prevent this kind of fix to be
> added as fuzzers find issues. Eventually, every supported release will
> be one using cbs where range checks are already implemented, so the
> quickest it's done the better.

Is there even any indication that this is an important fix that
requires it to be backported?