[FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

[wm4]

On Sat, 25 Oct 2014 21:51:25 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> This fixes the issue that a not set or not forwarded whitelist
> would allow to bypass it.
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavcodec/avcodec.h   |   17 +++++++++++++++++
>  libavcodec/utils.c     |   14 +++++++++++++-
>  libavformat/avformat.h |    4 ++++
>  libavformat/utils.c    |    6 ++++--
>  4 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index eac3fc7..1000c80 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3118,6 +3118,8 @@ typedef struct AVCodecContext {
>       * If NULL then all are allowed
>       * - encoding: unused
>       * - decoding: set by user through AVOPtions (NO direct access)
> +     *
> +     * @see av_enable_strict_whitelists()
>       */
>      char *codec_whitelist;
>  } AVCodecContext;
> @@ -5240,6 +5242,21 @@ const AVCodecDescriptor *avcodec_descriptor_next(const AVCodecDescriptor *prev);
>  const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
>  
>  /**
> + * Enables strict whitelists, so that if no whitelist is set nothing will be
> + * allowed.
> + * This improves security because when some code forgets to set or forward
> + * the whitelists it will fail instead of allowing an attacker to access a
> + * larger codebase than intended/needed.
> + */
> +void av_enable_strict_whitelists(void);
> +
> +/**
> + * returns non zero if strict whitelists are enabled.
> + * @see av_enable_strict_whitelists()
> + */
> +int av_are_strict_whitelists_enabled(void);
> +
> +/**
>   * @}
>   */
>  
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index b6ae1c0..6eb455a 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -118,6 +118,7 @@ volatile int ff_avcodec_locked;
>  static int volatile entangled_thread_counter = 0;
>  static void *codec_mutex;
>  static void *avformat_mutex;
> +static int strict_whitelists;

More global state...

>  
>  static inline int ff_fast_malloc(void *ptr, unsigned int *size, size_t min_size, int zero_realloc)
>  {
> @@ -157,6 +158,16 @@ void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size)
>          memset(*p, 0, min_size + FF_INPUT_BUFFER_PADDING_SIZE);
>  }
>  
> +void av_enable_strict_whitelists(void)
> +{
> +    strict_whitelists = 1;
> +}
> +
> +int av_are_strict_whitelists_enabled(void)
> +{
> +    return strict_whitelists;
> +}
> +
>  /* encoder management */
>  static AVCodec *first_avcodec = NULL;
>  static AVCodec **last_avcodec = &first_avcodec;
> @@ -1385,7 +1396,8 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>      if ((ret = av_opt_set_dict(avctx, &tmp)) < 0)
>          goto free_and_end;
>  
> -    if (avctx->codec_whitelist && av_match_list(codec->name, avctx->codec_whitelist, ',') <= 0) {
> +    if (   (avctx->codec_whitelist || av_are_strict_whitelists_enabled())
> +        && av_match_list(codec->name, avctx->codec_whitelist, ',') <= 0) {
>          av_log(avctx, AV_LOG_ERROR, "Codec (%s) not on whitelist\n", codec->name);
>          ret = AVERROR(EINVAL);
>          goto free_and_end;
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index f21a1d6..529b068 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1589,6 +1589,8 @@ typedef struct AVFormatContext {
>       * If NULL then all are allowed
>       * - encoding: unused
>       * - decoding: set by user through AVOptions (NO direct access)
> +     *
> +     * @see av_enable_strict_whitelists()
>       */
>      char *codec_whitelist;
>  
> @@ -1597,6 +1599,8 @@ typedef struct AVFormatContext {
>       * If NULL then all are allowed
>       * - encoding: unused
>       * - decoding: set by user through AVOptions (NO direct access)
> +     *
> +     * @see av_enable_strict_whitelists()
>       */
>      char *format_whitelist;
>  
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 61421c0..f8d5c88 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -304,7 +304,8 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st,
>  int av_demuxer_open(AVFormatContext *ic) {
>      int err;
>  
> -    if (ic->format_whitelist && av_match_list(ic->iformat->name, ic->format_whitelist, ',') <= 0) {
> +    if (   (ic->format_whitelist || av_are_strict_whitelists_enabled())
> +        && av_match_list(ic->iformat->name, ic->format_whitelist, ',') <= 0) {
>          av_log(ic, AV_LOG_ERROR, "Format not on whitelist\n");
>          return AVERROR(EINVAL);
>      }
> @@ -421,7 +422,8 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>          goto fail;
>      s->probe_score = ret;
>  
> -    if (s->format_whitelist && av_match_list(s->iformat->name, s->format_whitelist, ',') <= 0) {
> +    if (   (s->format_whitelist || av_are_strict_whitelists_enabled())
> +        && av_match_list(s->iformat->name, s->format_whitelist, ',') <= 0) {
>          av_log(s, AV_LOG_ERROR, "Format not on whitelist\n");
>          ret = AVERROR(EINVAL);
>          goto fail;