CVE-2017-14646: Out-of-bounds Read (CWE-125)
value: The AP4_AvccAtom and AP4_HvccAtom classes in Bento4 version 1.5.0-617 do not properly validate data sizes, leading to a heap-based buffer over-read and application crash in AP4_DataBuffer::SetData in Core/Ap4DataBuffer.cpp.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.5
Published: 2017-09-21T17:29Z
Repository: https://github.com/axiomatic-systems/Bento4
Language: C++
Files changed: Source/C++/Core/Ap4AvccAtom.cpp, Source/C++/Core/Ap4HvccAtom.cpp
Commit date: 2017-09-09 15:56:56 -0700
Commit message: fix for #188

Diff: diff --git a/Source/C++/Core/Ap4AvccAtom.cpp b/Source/C++/Core/Ap4AvccAtom.cpp
index d14f9a6a..4e55c8d2 100644
--- a/Source/C++/Core/Ap4AvccAtom.cpp
+++ b/Source/C++/Core/Ap4AvccAtom.cpp
@@ -150,22 +150,32 @@ AP4_AvccAtom::AP4_AvccAtom(AP4_UI32 size, const AP4_UI08* payload) :
     m_NaluLengthSize       = 1+(payload[4]&3);
     AP4_UI08 num_seq_params = payload[5]&31;
     m_SequenceParameters.EnsureCapacity(num_seq_params);
     unsigned int cursor = 6;
     for (unsigned int i=0; i<num_seq_params; i++) {
-        m_SequenceParameters.Append(AP4_DataBuffer());
-        AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
-        m_SequenceParameters[i].SetData(&payload[cursor]+2, param_length);
-        cursor += 2+param_length;
+        if (cursor+2 <= payload_size) {
+            AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
+            cursor += 2;
+            if (cursor + param_length < payload_size) {
+                m_SequenceParameters.Append(AP4_DataBuffer());
+                m_SequenceParameters[i].SetData(&payload[cursor], param_length);
+                cursor += param_length;
+            }
+        }
     }
     AP4_UI08 num_pic_params = payload[cursor++];
     m_PictureParameters.EnsureCapacity(num_pic_params);
     for (unsigned int i=0; i<num_pic_params; i++) {
-        m_PictureParameters.Append(AP4_DataBuffer());
-        AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
-        m_PictureParameters[i].SetData(&payload[cursor]+2, param_length);
-        cursor += 2+param_length;
+        if (cursor+2 <= payload_size) {
+            AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
+            cursor += 2;
+            if (cursor + param_length < payload_size) {
+                m_PictureParameters.Append(AP4_DataBuffer());
+                m_PictureParameters[i].SetData(&payload[cursor], param_length);
+                cursor += param_length;
+            }
+        }
     }
 }
 
 
 /*----------------------------------------------------------------------
diff --git a/Source/C++/Core/Ap4HvccAtom.cpp b/Source/C++/Core/Ap4HvccAtom.cpp
index 6c7f6dda..f8a8ecc8 100644
--- a/Source/C++/Core/Ap4HvccAtom.cpp
+++ b/Source/C++/Core/Ap4HvccAtom.cpp
@@ -245,10 +245,13 @@ AP4_HvccAtom::AP4_HvccAtom(AP4_UI08                         general_profile_spac
 AP4_HvccAtom::AP4_HvccAtom(AP4_UI32 size, const AP4_UI08* payload) :
     AP4_Atom(AP4_ATOM_TYPE_HVCC, size)
 {
     // make a copy of our configuration bytes
     unsigned int payload_size = size-AP4_ATOM_HEADER_SIZE;
+
+    // keep a raw copy
+    if (payload_size < 22) return;
     m_RawBytes.SetData(payload, payload_size);
 
     // parse the payload
     m_ConfigurationVersion   = payload[0];
     m_GeneralProfileSpace    = (payload[1]>>6) & 0x03;

Vulnerable code:         m_SequenceParameters.Append(AP4_DataBuffer());
        AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
        m_SequenceParameters[i].SetData(&payload[cursor]+2, param_length);
        cursor += 2+param_length;
        m_PictureParameters.Append(AP4_DataBuffer());
        AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
        m_PictureParameters[i].SetData(&payload[cursor]+2, param_length);
        cursor += 2+param_length;
Fixed code:         if (cursor+2 <= payload_size) {
            AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
            cursor += 2;
            if (cursor + param_length < payload_size) {
                m_SequenceParameters.Append(AP4_DataBuffer());
                m_SequenceParameters[i].SetData(&payload[cursor], param_length);
                cursor += param_length;
            }
        }
        if (cursor+2 <= payload_size) {
            AP4_UI16 param_length = AP4_BytesToInt16BE(&payload[cursor]);
            cursor += 2;
            if (cursor + param_length < payload_size) {
                m_PictureParameters.Append(AP4_DataBuffer());
                m_PictureParameters[i].SetData(&payload[cursor], param_length);
                cursor += param_length;
            }
        }

    // keep a raw copy
    if (payload_size < 22) return;