[FFmpeg-devel] [PATCH] xface: Fix encoder crashes due to too small on-stack array.

[Stefano Sabatini]

On date Saturday 2014-11-22 23:14:45 +0100, Reimar Döffinger encoded:
> Also add a FATE test.
> 
> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> ---
>  libavcodec/xface.h             | 9 +++++----
>  libavcodec/xfaceenc.c          | 3 +++
>  libavformat/nut.c              | 1 +
>  tests/fate/vcodec.mak          | 5 +++++
>  tests/ref/vsynth/vsynth1-xface | 4 ++++
>  tests/ref/vsynth/vsynth2-xface | 4 ++++
>  tests/ref/vsynth/vsynth3-xface | 4 ++++
>  7 files changed, 26 insertions(+), 4 deletions(-)
>  create mode 100644 tests/ref/vsynth/vsynth1-xface
>  create mode 100644 tests/ref/vsynth/vsynth2-xface
>  create mode 100644 tests/ref/vsynth/vsynth3-xface
> 
> diff --git a/libavcodec/xface.h b/libavcodec/xface.h
> index cd59ba0..6fbe908 100644
> --- a/libavcodec/xface.h
> +++ b/libavcodec/xface.h
> @@ -40,11 +40,12 @@
>  
>  /*
>   * Image is encoded as a big integer, using characters from '~' to
> - * '!', for a total of 92 symbols. In order to express 48x48=2304
> - * bits, we need a total of 354 digits, as given by:
> - * ceil(lg_92(2^2304)) = 354
> + * '!', for a total of 94 symbols. In order to express
> + * 48x48*2=8*XFACE_MAX_WORDS=4608
> + * bits, we need a total of 704 digits, as given by:
> + * ceil(lg_94(2^4608)) = 704
>   */
> -#define XFACE_MAX_DIGITS 354
> +#define XFACE_MAX_DIGITS 704
>
>  #define XFACE_BITSPERWORD 8
>  #define XFACE_WORDCARRY (1 << XFACE_BITSPERWORD)
> diff --git a/libavcodec/xfaceenc.c b/libavcodec/xfaceenc.c
> index e213c9d..0ade302 100644
> --- a/libavcodec/xfaceenc.c
> +++ b/libavcodec/xfaceenc.c
> @@ -27,6 +27,7 @@
>  #include "xface.h"
>  #include "avcodec.h"
>  #include "internal.h"
> +#include "libavutil/avassert.h"
>  
>  typedef struct XFaceContext {
>      AVClass *class;
> @@ -196,9 +197,11 @@ static int xface_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>  
>      /* write the inverted big integer in b to intbuf */
>      i = 0;
> +    av_assert0(b.nb_words < XFACE_MAX_WORDS);
>      while (b.nb_words) {
>          uint8_t r;
>          ff_big_div(&b, XFACE_PRINTS, &r);
> +        av_assert0(i < sizeof(intbuf));
>          intbuf[i++] = r + XFACE_FIRST_PRINT;
>      }
>  
> diff --git a/libavformat/nut.c b/libavformat/nut.c
> index 9224a96..86a0301 100644
> --- a/libavformat/nut.c
> +++ b/libavformat/nut.c
> @@ -40,6 +40,7 @@ const AVCodecTag ff_nut_data_tags[] = {
>  };
>  
>  const AVCodecTag ff_nut_video_tags[] = {
> +    { AV_CODEC_ID_XFACE,            MKTAG('X', 'F', 'A', 'C') },
>      { AV_CODEC_ID_VP9,              MKTAG('V', 'P', '9', '0') },
>      { AV_CODEC_ID_RAWVIDEO,         MKTAG('R', 'G', 'B', 15 ) },
>      { AV_CODEC_ID_RAWVIDEO,         MKTAG('B', 'G', 'R', 15 ) },
> diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
> index c715071..803edec 100644
> --- a/tests/fate/vcodec.mak
> +++ b/tests/fate/vcodec.mak
> @@ -294,6 +294,11 @@ fate-vsynth%-wmv2:               ENCOPTS = -qscale 10
>  FATE_VCODEC-$(call ENCDEC, RAWVIDEO, AVI) += yuv
>  fate-vsynth%-yuv:                CODEC = rawvideo
>  
> +FATE_VCODEC-$(call ENCDEC, XFACE, NUT) += xface
> +fate-vsynth%-xface:              ENCOPTS = -s 48x48 -sws_flags neighbor+bitexact
> +fate-vsynth%-xface:              DECOPTS = -sws_flags neighbor+bitexact
> +fate-vsynth%-xface:              FMT = nut
> +
>  FATE_VCODEC-$(call ENCDEC, YUV4, AVI) += yuv4
>  
>  FATE_VCODEC-$(call ENCDEC, Y41P, AVI) += y41p
> diff --git a/tests/ref/vsynth/vsynth1-xface b/tests/ref/vsynth/vsynth1-xface
> new file mode 100644
> index 0000000..3b916c6
> --- /dev/null
> +++ b/tests/ref/vsynth/vsynth1-xface
> @@ -0,0 +1,4 @@
> +487c3e53249f7b9f16e04257295998de *tests/data/fate/vsynth1-xface.nut
> +19746 tests/data/fate/vsynth1-xface.nut
> +42d8261bb538b8789840ac085f7fc4d2 *tests/data/fate/vsynth1-xface.out.rawvideo
> +stddev:  103.88 PSNR:  7.80 MAXDIFF:  254 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth2-xface b/tests/ref/vsynth/vsynth2-xface
> new file mode 100644
> index 0000000..5f60d66
> --- /dev/null
> +++ b/tests/ref/vsynth/vsynth2-xface
> @@ -0,0 +1,4 @@
> +6a1a7b467eeab2795510e7dd1ca528ff *tests/data/fate/vsynth2-xface.nut
> +17504 tests/data/fate/vsynth2-xface.nut
> +6d87881d630439d02c7a97f468d67a1c *tests/data/fate/vsynth2-xface.out.rawvideo
> +stddev:   99.01 PSNR:  8.22 MAXDIFF:  238 bytes:  7603200/  7603200
> diff --git a/tests/ref/vsynth/vsynth3-xface b/tests/ref/vsynth/vsynth3-xface
> new file mode 100644
> index 0000000..f98a5c5
> --- /dev/null
> +++ b/tests/ref/vsynth/vsynth3-xface
> @@ -0,0 +1,4 @@
> +f399a6b312d0a2d873b8a3bc761c5eba *tests/data/fate/vsynth3-xface.nut
> +15696 tests/data/fate/vsynth3-xface.nut
> +eafdc027c9c36f96e71e91a5682a0d2e *tests/data/fate/vsynth3-xface.out.rawvideo
> +stddev:   97.22 PSNR:  8.37 MAXDIFF:  236 bytes:    86700/    86700

LGTM, thanks.
-- 
FFmpeg = Fiendish & Frenzy Minimal Picky Enhancing Generator