[FFmpeg-devel] [PATCH 1/2] avcodec/wrapped_avframe: implement wrapped_avframe decoder

[Muhammad Faiz]

On Mon, Nov 9, 2015 at 8:22 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Mon, 9 Nov 2015 08:03:54 -0800
> Muhammad Faiz <mfcc64 at gmail.com> wrote:
>
>> From 4dcbda2e585404d2d79d5afcdc13fcb699f6f158 Mon Sep 17 00:00:00 2001
>> From: Muhammad Faiz <mfcc64 at gmail.com>
>> Date: Mon, 9 Nov 2015 15:55:13 +0700
>> Subject: [PATCH 1/2] avcodec/wrapped_avframe: implement wrapped_avframe
>>  decoder
>>
>> fix ticket #4985
>> for use in avdevice/lavfi
>> ---
>>  libavcodec/Makefile          |  1 +
>>  libavcodec/allcodecs.c       |  2 +-
>>  libavcodec/version.h         |  2 +-
>>  libavcodec/wrapped_avframe.c | 32 ++++++++++++++++++++++++++++++++
>>  4 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 68a573f..c60d512 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -577,6 +577,7 @@ OBJS-$(CONFIG_WMV2_ENCODER)            += wmv2enc.o wmv2.o \
>>                                            msmpeg4.o msmpeg4enc.o msmpeg4data.o
>>  OBJS-$(CONFIG_WNV1_DECODER)            += wnv1.o
>>  OBJS-$(CONFIG_WS_SND1_DECODER)         += ws-snd1.o
>> +OBJS-$(CONFIG_WRAPPED_AVFRAME_DECODER) += wrapped_avframe.o
>>  OBJS-$(CONFIG_WRAPPED_AVFRAME_ENCODER) += wrapped_avframe.o
>>  OBJS-$(CONFIG_XAN_DPCM_DECODER)        += dpcm.o
>>  OBJS-$(CONFIG_XAN_WC3_DECODER)         += xan.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 9f60d7c..3260927 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -342,7 +342,7 @@ void avcodec_register_all(void)
>>      REGISTER_DECODER(VP9,               vp9);
>>      REGISTER_DECODER(VQA,               vqa);
>>      REGISTER_DECODER(WEBP,              webp);
>> -    REGISTER_ENCODER(WRAPPED_AVFRAME,   wrapped_avframe);
>> +    REGISTER_ENCDEC (WRAPPED_AVFRAME,   wrapped_avframe);
>>      REGISTER_ENCDEC (WMV1,              wmv1);
>>      REGISTER_ENCDEC (WMV2,              wmv2);
>>      REGISTER_DECODER(WMV3,              wmv3);
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index 1e21f15..5eecf5b 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -29,7 +29,7 @@
>>  #include "libavutil/version.h"
>>
>>  #define LIBAVCODEC_VERSION_MAJOR  57
>> -#define LIBAVCODEC_VERSION_MINOR  15
>> +#define LIBAVCODEC_VERSION_MINOR  16
>>  #define LIBAVCODEC_VERSION_MICRO 100
>>
>>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>> diff --git a/libavcodec/wrapped_avframe.c b/libavcodec/wrapped_avframe.c
>> index 13c8d8a..185a1a2 100644
>> --- a/libavcodec/wrapped_avframe.c
>> +++ b/libavcodec/wrapped_avframe.c
>> @@ -32,6 +32,8 @@
>>  #include "libavutil/buffer.h"
>>  #include "libavutil/pixdesc.h"
>>
>> +#if CONFIG_WRAPPED_AVFRAME_ENCODER
>> +
>>  static void wrapped_avframe_release_buffer(void *unused, uint8_t *data)
>>  {
>>      AVFrame *frame = (AVFrame *)data;
>> @@ -71,3 +73,33 @@ AVCodec ff_wrapped_avframe_encoder = {
>>      .encode2        = wrapped_avframe_encode,
>>      .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
>>  };
>> +
>> +#endif
>> +
>> +#if CONFIG_WRAPPED_AVFRAME_DECODER
>> +
>> +static int wrapped_avframe_decode(AVCodecContext *avctx, void *data,
>> +                                  int *got_frame, AVPacket *avpkt)
>> +{
>> +    int ret;
>> +
>> +    if (avpkt->size != sizeof(AVFrame))
>> +        return AVERROR(EINVAL);
>> +
>> +    if ((ret = av_frame_ref((AVFrame *) data, (AVFrame *) avpkt->data)) < 0)
>> +        return ret;
>> +
>> +    *got_frame = 1;
>> +    return avpkt->size;
>> +}
>> +
>> +AVCodec ff_wrapped_avframe_decoder = {
>> +    .name           = "wrapped_avframe",
>> +    .long_name      = NULL_IF_CONFIG_SMALL("AVFrame to AVPacket passthrough"),
>> +    .type           = AVMEDIA_TYPE_VIDEO,
>> +    .id             = AV_CODEC_ID_WRAPPED_AVFRAME,
>> +    .decode         = wrapped_avframe_decode,
>> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
>> +};
>> +
>> +#endif
>
> This is very dangerous. You get potentially security relevant bad
> behavior if you get anyone to force a demuxer/decoder on an untrusted
> input file.

Yes, it is. The only check is packet size. But how can we be sure that
the underlying AVPacket contains proper AVFrame?
I think it can not, even with more checks. How can we check that
the buffer of AVFrame correctly point to memory? We can not check.

But as I know, the input file should be firstly demuxed, and the
only demuxer/device currently generate wrapped_avframe is
avdevice/lavfi (PATCH 2/2) which is trusted. Does FFmpeg allow
non wrapped_avframe packet be decoded
by wrapped_avframe decoder?

If it does not, I think this patch is safe.

If it does, of course this patch is unsafe. What is the solution?

Thank's.