On Tue, 03. Dec 23:30, Andreas Rheinhardt wrote:
> On Tue, Dec 3, 2019 at 10:41 PM Andriy Gelman <andriy.gel...@gmail.com>
> wrote:
>
> > From: Andriy Gelman <andriy.gel...@gmail.com>
> >
> > Check packet grow size against INT_MAX instead of SIZE_MAX.
> >
> > Found with libFuzzer:
> > 4294967044 cannot be represented as int.
> >
> > Signed-off-by: Andriy Gelman <andriy.gel...@gmail.com>
> > ---
> > libavcodec/hevc_mp4toannexb_bsf.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > b/libavcodec/hevc_mp4toannexb_bsf.c
> > index 09bce5b34c2..36fd6f0b15c 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -152,8 +152,8 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> > AVPacket *out)
> > extra_size = add_extradata * ctx->par_out->extradata_size;
> > got_irap |= is_irap;
> >
> > - if (SIZE_MAX - nalu_size < 4 ||
> > - SIZE_MAX - 4 - nalu_size < extra_size) {
> > + if (INT_MAX < 4 + nalu_size ||
> > + INT_MAX - 4 < extra_size + nalu_size) {
> > ret = AVERROR_INVALIDDATA;
> > goto fail;
> > }
> > --
> >
> > 1. Typo in "Interger".
> 2. You did not really fix the overflow problem: If nalu_size is (say)
> UINT32_MAX, 4 + nalu_size is 3 (no undefined behaviour involved here) and
> if extra_size is > 0 and <= INT_MAX - 4, then we don't go to fail here.
> Your check merely ensures that (after possible (defined) overflow) 4 +
> nalu_size + extra_size is >= 0 and <= INT_MAX, so that the automatic
> conversion to int (the type of the parameter of av_grow_packet()) is
> lossless. But in a scenario outlined here, the memcpy operations that
> follow will surely segfault.
thanks, I've resent.
> 3. There is another problem here: There is no check that the NAL unit is
> actually contained in the packet. A packet of size 10 bytes could contain a
> NAL unit with a size field indicating a size of (say) 1 GB and it would be
> allocated; it would not segfault, though, because the safe version of the
> bytestream2 API is used. (I think I already told you this, didn't I?)
Yes true, but I didn't make this problem. You are welcome to send the patch.
Thanks for looking over,
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
