diff mbox series

[FFmpeg-devel,01/12] ffprobe: only hash extradata when it is present

Message ID 20210425070320.14197-1-anton@khirnov.net
State Accepted
Commit 544631cab1140044524bf6bc04ec8c5c275627b6
Headers show
Series [FFmpeg-devel,01/12] ffprobe: only hash extradata when it is present | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Anton Khirnov April 25, 2021, 7:03 a.m. UTC
Passing zero-sized/NULL buffers to av_hash_update() is invalid and may
crash with certain hashes.
---
 fftools/ffprobe.c                                         | 7 +++++--
 tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 -
 tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov         | 1 -
 3 files changed, 5 insertions(+), 4 deletions(-)

Comments

Andreas Rheinhardt April 26, 2021, 11:38 a.m. UTC | #1
Anton Khirnov:
> Passing zero-sized/NULL buffers to av_hash_update() is invalid and may
> crash with certain hashes.
> ---
>  fftools/ffprobe.c                                         | 7 +++++--
>  tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 -
>  tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov         | 1 -
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index a2cb7dc986..9039985dcb 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -2741,8 +2741,11 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
>      if (do_show_data)
>          writer_print_data(w, "extradata", par->extradata,
>                                            par->extradata_size);
> -    writer_print_data_hash(w, "extradata_hash", par->extradata,
> -                                                par->extradata_size);
> +
> +    if (par->extradata_size > 0) {
> +        writer_print_data_hash(w, "extradata_hash", par->extradata,
> +                                                    par->extradata_size);
> +    }
>  
>      /* Print disposition information */
>  #define PRINT_DISPOSITION(flagname, name) do {                                \
> diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> index b69181ace7..4ef569df89 100644
> --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> @@ -50,7 +50,6 @@ bits_per_raw_sample=N/A
>  nb_frames=1
>  nb_read_frames=N/A
>  nb_read_packets=1
> -extradata_hash=adler32:00000001
>  DISPOSITION:default=1
>  DISPOSITION:dub=0
>  DISPOSITION:original=0
> diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> index 16923e8684..70e7cdc943 100644
> --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> @@ -50,7 +50,6 @@ bits_per_raw_sample=N/A
>  nb_frames=1
>  nb_read_frames=N/A
>  nb_read_packets=1
> -extradata_hash=adler32:00000001
>  DISPOSITION:default=1
>  DISPOSITION:dub=0
>  DISPOSITION:original=0
> 
Which hashes crash when size is zero? Which do so when extradata is NULL
with size == 0?

- Andreas
Anton Khirnov April 26, 2021, 11:49 a.m. UTC | #2
Quoting Andreas Rheinhardt (2021-04-26 13:38:14)
> Anton Khirnov:
> > Passing zero-sized/NULL buffers to av_hash_update() is invalid and may
> > crash with certain hashes.
> > ---
> >  fftools/ffprobe.c                                         | 7 +++++--
> >  tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 -
> >  tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov         | 1 -
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index a2cb7dc986..9039985dcb 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -2741,8 +2741,11 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
> >      if (do_show_data)
> >          writer_print_data(w, "extradata", par->extradata,
> >                                            par->extradata_size);
> > -    writer_print_data_hash(w, "extradata_hash", par->extradata,
> > -                                                par->extradata_size);
> > +
> > +    if (par->extradata_size > 0) {
> > +        writer_print_data_hash(w, "extradata_hash", par->extradata,
> > +                                                    par->extradata_size);
> > +    }
> >  
> >      /* Print disposition information */
> >  #define PRINT_DISPOSITION(flagname, name) do {                                \
> > diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > index b69181ace7..4ef569df89 100644
> > --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > @@ -50,7 +50,6 @@ bits_per_raw_sample=N/A
> >  nb_frames=1
> >  nb_read_frames=N/A
> >  nb_read_packets=1
> > -extradata_hash=adler32:00000001
> >  DISPOSITION:default=1
> >  DISPOSITION:dub=0
> >  DISPOSITION:original=0
> > diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > index 16923e8684..70e7cdc943 100644
> > --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > @@ -50,7 +50,6 @@ bits_per_raw_sample=N/A
> >  nb_frames=1
> >  nb_read_frames=N/A
> >  nb_read_packets=1
> > -extradata_hash=adler32:00000001
> >  DISPOSITION:default=1
> >  DISPOSITION:dub=0
> >  DISPOSITION:original=0
> > 
> Which hashes crash when size is zero? Which do so when extradata is NULL
> with size == 0?

Did not check in detail, but at least CRC32 crashes on NULL input. E.g.:
ffprobe -show_streams -show_data_hash CRC32 samples/hap/HAPQA_NoSnappy_127x1.mov
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index a2cb7dc986..9039985dcb 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2741,8 +2741,11 @@  static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
     if (do_show_data)
         writer_print_data(w, "extradata", par->extradata,
                                           par->extradata_size);
-    writer_print_data_hash(w, "extradata_hash", par->extradata,
-                                                par->extradata_size);
+
+    if (par->extradata_size > 0) {
+        writer_print_data_hash(w, "extradata_hash", par->extradata,
+                                                    par->extradata_size);
+    }
 
     /* Print disposition information */
 #define PRINT_DISPOSITION(flagname, name) do {                                \
diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
index b69181ace7..4ef569df89 100644
--- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
+++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
@@ -50,7 +50,6 @@  bits_per_raw_sample=N/A
 nb_frames=1
 nb_read_frames=N/A
 nb_read_packets=1
-extradata_hash=adler32:00000001
 DISPOSITION:default=1
 DISPOSITION:dub=0
 DISPOSITION:original=0
diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
index 16923e8684..70e7cdc943 100644
--- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
+++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
@@ -50,7 +50,6 @@  bits_per_raw_sample=N/A
 nb_frames=1
 nb_read_frames=N/A
 nb_read_packets=1
-extradata_hash=adler32:00000001
 DISPOSITION:default=1
 DISPOSITION:dub=0
 DISPOSITION:original=0