From 8a1c66e32a3175081aa8d0fb472b7757fcbc6a22 Mon Sep 17 00:00:00 2001 From: Dave O'Rourke Date: Tue, 19 Mar 2019 12:18:28 -0400 Subject: [PATCH 1/7] Throw exception when invalid atom size would cause integer underflow The calculation `hdrSize - dataSize` can underflow the 64-bit unsigned int dataSize type, which can lead to incorrect results. We throw an exception to stop the code from going any further. Addresses https://nvd.nist.gov/vuln/detail/CVE-2018-14325 --- src/mp4atom.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mp4atom.cpp b/src/mp4atom.cpp index 2d71f0d..d208c2c 100644 --- a/src/mp4atom.cpp +++ b/src/mp4atom.cpp @@ -143,6 +143,13 @@ MP4Atom* MP4Atom::ReadAtom(MP4File& file, MP4Atom* pParentAtom) dataSize = file.GetSize() - pos; } + // Prevent integer underflow due to incorrect atom size read from file + if ( dataSize < hdrSize ) { + ostringstream oss; + oss << "Invalid atom size in '" << type << "' atom, dataSize = " << dataSize << " cannot be less than hdrSize = " << static_cast( hdrSize ); + log.errorf( "%s: \"%s\": %s", __FUNCTION__, file.GetFilename().c_str(), oss.str().c_str() ); + throw new Exception( oss.str().c_str(), __FILE__, __LINE__, __FUNCTION__ ); + } dataSize -= hdrSize; log.verbose1f("\"%s\": type = \"%s\" data-size = %" PRIu64 " (0x%" PRIx64 ") hdr %u", From 596c5a5c170f934cab0e0430a45ca16cc6b76a14 Mon Sep 17 00:00:00 2001 From: Tom Sirgedas Date: Thu, 14 Mar 2019 14:23:17 -0400 Subject: [PATCH 2/7] fix vulnerability where an atom list size is enormous and calculating the number of bytes needed to hold the list overflows Addresses https://nvd.nist.gov/vuln/detail/CVE-2018-14326 and https://nvd.nist.gov/vuln/detail/CVE-2018-14446 --- src/mp4array.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mp4array.h b/src/mp4array.h index c49d59b..69d470a 100644 --- a/src/mp4array.h +++ b/src/mp4array.h @@ -102,6 +102,8 @@ class MP4Array { void Resize(MP4ArrayIndex newSize) { \ m_numElements = newSize; \ m_maxNumElements = newSize; \ + if ( (uint64_t) m_maxNumElements * sizeof(type) > 0xFFFFFFFF ) \ + throw new PlatformException("requested array size exceeds 4GB", ERANGE, __FILE__, __LINE__, __FUNCTION__); /* prevent overflow */ \ m_elements = (type*)MP4Realloc(m_elements, \ m_maxNumElements * sizeof(type)); \ } \ From e52476167838f7a189dfc23fd7bc353317b8802d Mon Sep 17 00:00:00 2001 From: Dave O'Rourke Date: Tue, 19 Mar 2019 13:21:45 -0400 Subject: [PATCH 3/7] Fix intellisense error due to missing space in ASSERT macro --- src/mp4util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mp4util.h b/src/mp4util.h index 1fbbd81..b33bb44 100644 --- a/src/mp4util.h +++ b/src/mp4util.h @@ -33,7 +33,7 @@ namespace mp4v2 { namespace impl { #ifndef ASSERT # define ASSERT(expr) \ if (!(expr)) { \ - throw new Exception("assert failure: "LIBMPV42_STRINGIFY((expr)), __FILE__, __LINE__, __FUNCTION__ ); \ + throw new Exception("assert failure: " LIBMPV42_STRINGIFY((expr)), __FILE__, __LINE__, __FUNCTION__ ); \ } #endif From c9ffd33fb8d7308de197b03beee48078e40750cb Mon Sep 17 00:00:00 2001 From: Dave O'Rourke Date: Tue, 19 Mar 2019 14:59:16 -0400 Subject: [PATCH 4/7] Don't allow an ilst atom to have an ilst child atom Addresses https://nvd.nist.gov/vuln/detail/CVE-2018-14379 --- src/mp4atom.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mp4atom.cpp b/src/mp4atom.cpp index d208c2c..1e4c440 100644 --- a/src/mp4atom.cpp +++ b/src/mp4atom.cpp @@ -785,8 +785,10 @@ MP4Atom::factory( MP4File &file, MP4Atom* parent, const char* type ) const char* const ptype = parent->GetType(); if( descendsFrom( parent, "ilst" )) { - if( ATOMID( ptype ) == ATOMID( "ilst" )) - return new MP4ItemAtom( file, type ); + if( ATOMID( ptype ) == ATOMID( "ilst" )) { + ASSERT( ATOMID( type ) != ATOMID( "ilst" )); // don't allow ilst to be a child of ilst + return new MP4ItemAtom( file, type ); + } if( ATOMID( type ) == ATOMID( "data" )) return new MP4DataAtom(file); From 6802fa495dfe1d87b544750b032ba84b4e492853 Mon Sep 17 00:00:00 2001 From: Dave O'Rourke Date: Tue, 19 Mar 2019 17:06:21 -0400 Subject: [PATCH 5/7] Check MP4NameFirstMatches loop termination condition more closely If the atom type has an embedded nul character "\x00", the loop can terminate early and return true (match) when it should return false (no match). This function should only return true if we have reached the loop termination conditions on s2. For example: // These test cases passed before and after this change AssertIsTrue( MP4NameFirstMatches( "sdtp", "sdtp" ) ); // Exact match AssertIsTrue( MP4NameFirstMatches( "trak", "trak[1]" ) ); // Matches up to [ AssertIsTrue( MP4NameFirstMatches( "sdtp", "sdtp." ) ); // Matches up to . AssertIsFalse( MP4NameFirstMatches( "\x00dtp", "sdtp." ) ); // Nul character at s[0] // These test cases failed before this change, and pass after this change AssertIsFalse( MP4NameFirstMatches( "s\x00tp", "sdtp." ) ); // Nul character at s[1] AssertIsFalse( MP4NameFirstMatches( "sd\x00p", "sdtp." ) ); // Nul character at s[2] AssertIsFalse( MP4NameFirstMatches( "sdt\x00", "sdtp." ) ); // Nul character at s[3] Addresses https://nvd.nist.gov/vuln/detail/CVE-2018-14403 --- src/mp4util.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mp4util.cpp b/src/mp4util.cpp index 47bd74e..8a915b2 100644 --- a/src/mp4util.cpp +++ b/src/mp4util.cpp @@ -46,6 +46,12 @@ bool MP4NameFirstMatches(const char* s1, const char* s2) s1++; s2++; } + + // Make sure we finished the loop by using up s2, not s1 + if ( *s2 != '[' && *s2 != '.' && *s2 != '\0' ) { + return false; + } + return true; } From 9bed0080661c71cc196767d7a97ee32d339fe2ec Mon Sep 17 00:00:00 2001 From: Dave O'Rourke Date: Wed, 20 Mar 2019 08:57:29 -0400 Subject: [PATCH 6/7] Null out pointer after free to prevent double free If an exception occurs (because of a crafted MP4) before the value is reassigned, then a double free can occur. By setting the pointer to NULL after the first free, we prevent the double free in this case. Addresses: https://nvd.nist.gov/vuln/detail/CVE-2018-14054 --- src/mp4property.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mp4property.cpp b/src/mp4property.cpp index dc1e2ed..d75f9e1 100644 --- a/src/mp4property.cpp +++ b/src/mp4property.cpp @@ -391,8 +391,10 @@ void MP4StringProperty::Read( MP4File& file, uint32_t index ) char*& value = m_values[i]; // Generally a default atom setting, e.g. see atom_avc1.cpp, "JVT/AVC Coding"; we'll leak this string if - // we don't free. Note that MP4Free checks for null. - MP4Free(value); + // we don't free. Note that this code checks for null before calling free and sets the pointer to null + // after freeing it, to prevent a double free in case an exception occurs before the value is reassigned. + MP4Free( value ); + value = NULL; if( m_useCountedFormat ) { value = file.ReadCountedString( (m_useUnicode ? 2 : 1), m_useExpandedCount, m_fixedLength ); From e9458829bef2f9fa5d666d0e599db31405177e88 Mon Sep 17 00:00:00 2001 From: Alex Novak Date: Mon, 8 Apr 2019 09:28:47 -0400 Subject: [PATCH 7/7] Bump version to 3.0.4 I copied what DRO did in his commit for the top of the branch (1695b60bcaf232c5bad15c5b5d42a3c231774098), except this one is branched off the last release for Camtasia 9. --- mp4v2-Win/include/mp4v2/project.h | 6 +++--- mp4v2-Win/mp4v2.autopkg | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/mp4v2-Win/include/mp4v2/project.h b/mp4v2-Win/include/mp4v2/project.h index 76bd6f8..4fc3732 100644 --- a/mp4v2-Win/include/mp4v2/project.h +++ b/mp4v2-Win/include/mp4v2/project.h @@ -6,17 +6,17 @@ #define MP4V2_PROJECT_name "MP4v2" #define MP4V2_PROJECT_name_lower "mp4v2" #define MP4V2_PROJECT_name_upper "MP4V2" -#define MP4V2_PROJECT_name_formal "MP4v2 3.0.3.0" +#define MP4V2_PROJECT_name_formal "MP4v2 3.0.4.0" #define MP4V2_PROJECT_url_website "http://code.google.com/p/mp4v2" #define MP4V2_PROJECT_url_downloads "http://code.google.com/p/mp4v2/downloads/list" #define MP4V2_PROJECT_url_discussion "http://groups.google.com/group/mp4v2" #define MP4V2_PROJECT_irc "irc://irc.freenode.net/handbrake" #define MP4V2_PROJECT_bugreport "" -#define MP4V2_PROJECT_version "3.0.3.0" +#define MP4V2_PROJECT_version "3.0.4.0" #define MP4V2_PROJECT_version_hex 0x00020100 #define MP4V2_PROJECT_version_major 3 #define MP4V2_PROJECT_version_minor 0 -#define MP4V2_PROJECT_version_point 3 +#define MP4V2_PROJECT_version_point 4 #define MP4V2_PROJECT_repo_url "https://mp4v2.googlecode.com/svn/trunk" #define MP4V2_PROJECT_repo_root "https://mp4v2.googlecode.com/svn" #define MP4V2_PROJECT_repo_uuid "6e6572fa-98a6-11dd-ad9f-f77439c74b79" diff --git a/mp4v2-Win/mp4v2.autopkg b/mp4v2-Win/mp4v2.autopkg index 71918bd..91b0acf 100644 --- a/mp4v2-Win/mp4v2.autopkg +++ b/mp4v2-Win/mp4v2.autopkg @@ -10,7 +10,7 @@ nuget nuspec { id = mp4v2; - version: 3.0.3.0; + version: 3.0.4.0; title: MP4v2 Library; authors: { TechSmith Corporation }; owners: { TechSmith Corporation }; @@ -27,7 +27,8 @@ nuget 3.0.1.0 Bumping the version number since some new functions were added in the past three commits 3.0.1.1 Sync version number in .autopkg with the one in mp4v2/project.h 3.0.2.0 Picking up Stephen Wagner's updates to handle PNG MOVs - 3.0.3.0 Fixing a bug where version number 3.0.2 was inconsistent in project.h"; + 3.0.3.0 Fixing a bug where version number 3.0.2 was inconsistent in project.h + 3.0.4.0 Security fixes"; copyright: ""; tags: { native, mp4v2, mp4, vs2015 }; };