On Thu, Jun 7, 2018 at 10:38 AM Jacob Trimble <modma...@google.com> wrote:
>
> Found by Chrome's ClusterFuzz: http://crbug.com/850389
>
> Signed-off-by: Jacob Trimble <modma...@google.com>
> ---
>  libavformat/mov.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 4ad19122b3..d07171b3f4 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -6041,6 +6041,11 @@ static int mov_read_saiz(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>      if (ret != 1)
>          return ret;
>
> +    if (!sc->cenc.default_encrypted_sample) {
> +        // Didn't see a 'schm' or 'tenc' atom, so it isn't encrypted.
> +        return 0;
> +    }
> +
>      if (encryption_index->nb_encrypted_samples) {
>          // This can happen if we have both saio/saiz and senc atoms.
>          av_log(c->fc, AV_LOG_DEBUG, "Ignoring duplicate encryption info in 
> saiz\n");
> @@ -6095,6 +6100,11 @@ static int mov_read_saio(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>      if (ret != 1)
>          return ret;
>
> +    if (!sc->cenc.default_encrypted_sample) {
> +        // Didn't see a 'schm' or 'tenc' atom, so it isn't encrypted.
> +        return 0;
> +    }
> +
>      if (encryption_index->nb_encrypted_samples) {
>          // This can happen if we have both saio/saiz and senc atoms.
>          av_log(c->fc, AV_LOG_DEBUG, "Ignoring duplicate encryption info in 
> saio\n");
> --
> 2.17.1.1185.g55be947832-goog
>

Based on comments downstream, I've added error checks for the
encrypted type of saio/saiz atoms.
From e4185c0fd08a1baedcf81935ff0f5ac9a97eba4e Mon Sep 17 00:00:00 2001
From: Jacob Trimble <modma...@google.com>
Date: Thu, 7 Jun 2018 10:29:33 -0700
Subject: [PATCH] avformat/mov: Fix reading saio/saiz for clear content.

This validates that the common encryption saio/saiz atoms only appear
when the data is actually encrypted.  This also ignores those atoms
in clear content.

Found by Chrome's ClusterFuzz: http://crbug.com/850389

Signed-off-by: Jacob Trimble <modma...@google.com>
---
 libavformat/mov.c | 71 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 4ad19122b3..2fca025889 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6035,7 +6035,7 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     MOVEncryptionIndex *encryption_index;
     MOVStreamContext *sc;
     int ret;
-    unsigned int sample_count;
+    unsigned int sample_count, aux_info_type, aux_info_param;
 
     ret = get_current_encryption_info(c, &encryption_index, &sc);
     if (ret != 1)
@@ -6054,14 +6054,33 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     avio_r8(pb); /* version */
     if (avio_rb24(pb) & 0x01) {  /* flags */
-        if (avio_rb32(pb) != sc->cenc.default_encrypted_sample->scheme) {
-            av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type\n");
-            return 0;
-        }
-        if (avio_rb32(pb) != 0) {
-            av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type_parameter\n");
-            return 0;
+        aux_info_type = avio_rb32(pb);
+        aux_info_param = avio_rb32(pb);
+        if (sc->cenc.default_encrypted_sample) {
+            if (aux_info_type != sc->cenc.default_encrypted_sample->scheme) {
+                av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type\n");
+                return 0;
+            }
+            if (aux_info_param != 0) {
+                av_log(c->fc, AV_LOG_DEBUG, "Ignoring saiz box with non-zero aux_info_type_parameter\n");
+                return 0;
+            }
+        } else {
+            // Didn't see 'schm' or 'tenc', so this isn't encrypted.
+            if ((aux_info_type == MKBETAG('c','e','n','c') ||
+                 aux_info_type == MKBETAG('c','e','n','s') ||
+                 aux_info_type == MKBETAG('c','b','c','1') ||
+                 aux_info_type == MKBETAG('c','b','c','s')) &&
+                aux_info_param == 0) {
+                av_log(c->fc, AV_LOG_ERROR, "Saw encrypted saiz without schm/tenc\n");
+                return AVERROR_INVALIDDATA;
+            } else {
+                return 0;
+            }
         }
+    } else if (!sc->cenc.default_encrypted_sample) {
+        // Didn't see 'schm' or 'tenc', so this isn't encrypted.
+        return 0;
     }
 
     encryption_index->auxiliary_info_default_size = avio_r8(pb);
@@ -6089,7 +6108,8 @@ static int mov_read_saio(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     MOVEncryptionIndex *encryption_index;
     MOVStreamContext *sc;
     int i, ret;
-    unsigned int version, entry_count, alloc_size = 0;
+    unsigned int version, entry_count, aux_info_type, aux_info_param;
+    unsigned int alloc_size = 0;
 
     ret = get_current_encryption_info(c, &encryption_index, &sc);
     if (ret != 1)
@@ -6108,14 +6128,33 @@ static int mov_read_saio(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     version = avio_r8(pb); /* version */
     if (avio_rb24(pb) & 0x01) {  /* flags */
-        if (avio_rb32(pb) != sc->cenc.default_encrypted_sample->scheme) {
-            av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type\n");
-            return 0;
-        }
-        if (avio_rb32(pb) != 0) {
-            av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type_parameter\n");
-            return 0;
+        aux_info_type = avio_rb32(pb);
+        aux_info_param = avio_rb32(pb);
+        if (sc->cenc.default_encrypted_sample) {
+            if (aux_info_type != sc->cenc.default_encrypted_sample->scheme) {
+                av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type\n");
+                return 0;
+            }
+            if (aux_info_param != 0) {
+                av_log(c->fc, AV_LOG_DEBUG, "Ignoring saio box with non-zero aux_info_type_parameter\n");
+                return 0;
+            }
+        } else {
+            // Didn't see 'schm' or 'tenc', so this isn't encrypted.
+            if ((aux_info_type == MKBETAG('c','e','n','c') ||
+                 aux_info_type == MKBETAG('c','e','n','s') ||
+                 aux_info_type == MKBETAG('c','b','c','1') ||
+                 aux_info_type == MKBETAG('c','b','c','s')) &&
+                aux_info_param == 0) {
+                av_log(c->fc, AV_LOG_ERROR, "Saw encrypted saio without schm/tenc\n");
+                return AVERROR_INVALIDDATA;
+            } else {
+                return 0;
+            }
         }
+    } else if (!sc->cenc.default_encrypted_sample) {
+        // Didn't see 'schm' or 'tenc', so this isn't encrypted.
+        return 0;
     }
 
     entry_count = avio_rb32(pb);
-- 
2.17.1.1185.g55be947832-goog

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to