diff mbox

[FFmpeg-devel,4/4] avcodec/dirac_parser: Fix overflow in dts

Message ID 20190711214940.13431-4-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer July 11, 2019, 9:49 p.m. UTC
Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
Fixes: 15568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5634719611355136

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

Comments

James Almer July 11, 2019, 11:58 p.m. UTC | #1
On 7/11/2019 6:49 PM, Michael Niedermayer wrote:
> Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
> Fixes: 15568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5634719611355136
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/dirac_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/dirac_parser.c b/libavcodec/dirac_parser.c
> index 1ade44a438..8722ef17b7 100644
> --- a/libavcodec/dirac_parser.c
> +++ b/libavcodec/dirac_parser.c
> @@ -214,7 +214,7 @@ static int dirac_combine_frame(AVCodecParserContext *s, AVCodecContext *avctx,
>                                pc->index - 13 - pu1.prev_pu_offset;
>              int pts = AV_RB32(cur_pu + 13);
>              if (s->last_pts == 0 && s->last_dts == 0)
> -                s->dts = pts - 1;
> +                s->dts = pts - 1LL;

Unless that AV_RB32() value can be negative in valid bitstreams, just
make pts int64_t instead. That's the type for both pts and dts in
AVCodecParserContext.

>              else
>                  s->dts = s->last_dts + 1;
>              s->pts = pts;
>
Michael Niedermayer July 12, 2019, 5:51 p.m. UTC | #2
On Thu, Jul 11, 2019 at 08:58:50PM -0300, James Almer wrote:
> On 7/11/2019 6:49 PM, Michael Niedermayer wrote:
> > Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
> > Fixes: 15568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5634719611355136
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/dirac_parser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/dirac_parser.c b/libavcodec/dirac_parser.c
> > index 1ade44a438..8722ef17b7 100644
> > --- a/libavcodec/dirac_parser.c
> > +++ b/libavcodec/dirac_parser.c
> > @@ -214,7 +214,7 @@ static int dirac_combine_frame(AVCodecParserContext *s, AVCodecContext *avctx,
> >                                pc->index - 13 - pu1.prev_pu_offset;
> >              int pts = AV_RB32(cur_pu + 13);
> >              if (s->last_pts == 0 && s->last_dts == 0)
> > -                s->dts = pts - 1;
> > +                s->dts = pts - 1LL;
> 
> Unless that AV_RB32() value can be negative in valid bitstreams, just
> make pts int64_t instead. That's the type for both pts and dts in
> AVCodecParserContext.

ill try and will change to that if i spot nothing breaking but libdirac 1.0.2
seems to use plain int for this so on most platforms that would not result
in positive numbers once the highest bit is set ...
I cant find a clear statment in the spec about this but my feeling is
that unsigened was the intent
i suspect this question doesnt affect real world content

anyone knows if dirac is still maintained somewhere ? 
(if so ill report this minor issue there if it appears to affect the curent 
 source still ...)

Thanks


[...]
diff mbox

Patch

diff --git a/libavcodec/dirac_parser.c b/libavcodec/dirac_parser.c
index 1ade44a438..8722ef17b7 100644
--- a/libavcodec/dirac_parser.c
+++ b/libavcodec/dirac_parser.c
@@ -214,7 +214,7 @@  static int dirac_combine_frame(AVCodecParserContext *s, AVCodecContext *avctx,
                               pc->index - 13 - pu1.prev_pu_offset;
             int pts = AV_RB32(cur_pu + 13);
             if (s->last_pts == 0 && s->last_dts == 0)
-                s->dts = pts - 1;
+                s->dts = pts - 1LL;
             else
                 s->dts = s->last_dts + 1;
             s->pts = pts;