From 06f0d062d8329d7cc966946a10df0a3e857d5187 Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Fri, 22 Mar 2024 22:10:49 -0700 Subject: [PATCH 1/6] Add boundary checks to Ap4OdheAtom Fuzzer caught another large malloc. This is caused by lack of boundary check in Ap4OdheAtom causing underflow. --- Source/C++/Core/Ap4OdheAtom.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/C++/Core/Ap4OdheAtom.cpp b/Source/C++/Core/Ap4OdheAtom.cpp index 98fd74af7..fbedc49f5 100644 --- a/Source/C++/Core/Ap4OdheAtom.cpp +++ b/Source/C++/Core/Ap4OdheAtom.cpp @@ -64,9 +64,11 @@ AP4_OdheAtom::AP4_OdheAtom(AP4_UI32 size, AP4_AtomFactory& atom_factory) : AP4_ContainerAtom(AP4_ATOM_TYPE_ODHE, size, false, version, flags) { + if (size < AP4_FULL_ATOM_HEADER_SIZE+1) return; // read the content type AP4_UI08 content_type_length; stream.ReadUI08(content_type_length); + if (size < AP4_FULL_ATOM_HEADER_SIZE+1+content_type_length) return; char content_type[256]; stream.Read(content_type, content_type_length); m_ContentType.Assign(content_type, content_type_length); From 84b95d424c27d1dd7ec503377cab392a9434e8fb Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Fri, 22 Mar 2024 22:16:17 -0700 Subject: [PATCH 2/6] More boundary checks for Ap4SaioAtom Earlier boundary checks were insufficient to prevent certain potential payloads. This adds more boundary checks to prevent underflow of remains. I have also remove the usage of GetHeaderSize in constructor as it is a virtual method. --- Source/C++/Core/Ap4SaioAtom.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/C++/Core/Ap4SaioAtom.cpp b/Source/C++/Core/Ap4SaioAtom.cpp index 298a4e969..05ace6401 100644 --- a/Source/C++/Core/Ap4SaioAtom.cpp +++ b/Source/C++/Core/Ap4SaioAtom.cpp @@ -97,12 +97,14 @@ AP4_SaioAtom::AP4_SaioAtom(AP4_UI32 size, m_AuxInfoType(0), m_AuxInfoTypeParameter(0) { - AP4_UI32 remains = size-GetHeaderSize(); + AP4_SI32 remains = size-AP4_FULL_ATOM_HEADER_SIZE; if (flags & 1) { + if (remains < 8) return; stream.ReadUI32(m_AuxInfoType); stream.ReadUI32(m_AuxInfoTypeParameter); remains -= 8; } + if (remains < 4) return; AP4_UI32 entry_count = 0; AP4_Result result = stream.ReadUI32(entry_count); if (AP4_FAILED(result)) return; From f9f6b22cffde31713e8ee193b74717f07e8626f1 Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Sat, 23 Mar 2024 23:17:44 -0700 Subject: [PATCH 3/6] Add boundary checks to Ap4SbgpAtom Fuzzer caught another large malloc. This is caused by lack of boundary check in Ap4SbgpAtom causing underflow. --- Source/C++/Core/Ap4SbgpAtom.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Source/C++/Core/Ap4SbgpAtom.cpp b/Source/C++/Core/Ap4SbgpAtom.cpp index 58d3190d2..ab815bf9d 100644 --- a/Source/C++/Core/Ap4SbgpAtom.cpp +++ b/Source/C++/Core/Ap4SbgpAtom.cpp @@ -73,13 +73,16 @@ AP4_SbgpAtom::AP4_SbgpAtom(AP4_UI32 size, m_GroupingType(0), m_GroupingTypeParameter(0) { - AP4_UI32 remains = size-GetHeaderSize(); + if (size < AP4_FULL_ATOM_HEADER_SIZE + 4) return; + AP4_UI32 remains = size-AP4_FULL_ATOM_HEADER_SIZE; stream.ReadUI32(m_GroupingType); remains -= 4; if (version >= 1) { + if (remains < 4) return; stream.ReadUI32(m_GroupingTypeParameter); remains -= 4; } + if (remains < 4) return; AP4_UI32 entry_count = 0; AP4_Result result = stream.ReadUI32(entry_count); if (AP4_FAILED(result)) return; From 8806fe2f690f4a9a0ed072d846a1320778b09192 Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Tue, 26 Mar 2024 17:16:38 -0700 Subject: [PATCH 4/6] Prevent overflow in boundary check for Ap4SbgpAtom Fuzzer caught another large malloc in Ap4SbgpAtom. It is caused by overflow in boundary check --- Source/C++/Core/Ap4SbgpAtom.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/C++/Core/Ap4SbgpAtom.cpp b/Source/C++/Core/Ap4SbgpAtom.cpp index ab815bf9d..ff2859863 100644 --- a/Source/C++/Core/Ap4SbgpAtom.cpp +++ b/Source/C++/Core/Ap4SbgpAtom.cpp @@ -87,7 +87,7 @@ AP4_SbgpAtom::AP4_SbgpAtom(AP4_UI32 size, AP4_Result result = stream.ReadUI32(entry_count); if (AP4_FAILED(result)) return; remains -= 4; - if (remains < entry_count*8) { + if (remains < (AP4_UI64)entry_count*8) { return; } m_Entries.SetItemCount(entry_count); From b977973f3337aed0d8450dfac331970c66496b42 Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Wed, 27 Mar 2024 23:37:27 -0700 Subject: [PATCH 5/6] Boundary checks in Ap4ContainerAtom The lack of boundary checks in Ap4ContainerAtom leads to an underflow in size which then leads to existing validation checks to fail and hence allow large malloc. --- Source/C++/Core/Ap4ContainerAtom.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/C++/Core/Ap4ContainerAtom.cpp b/Source/C++/Core/Ap4ContainerAtom.cpp index d8c800895..67bc9dfd8 100644 --- a/Source/C++/Core/Ap4ContainerAtom.cpp +++ b/Source/C++/Core/Ap4ContainerAtom.cpp @@ -136,6 +136,7 @@ AP4_ContainerAtom::AP4_ContainerAtom(Type type, AP4_AtomFactory& atom_factory) : AP4_Atom(type, size, force_64) { + if (size < GetHeaderSize()) return; ReadChildren(atom_factory, stream, size-GetHeaderSize()); } @@ -151,6 +152,7 @@ AP4_ContainerAtom::AP4_ContainerAtom(Type type, AP4_AtomFactory& atom_factory) : AP4_Atom(type, size, force_64, version, flags) { + if (size < GetHeaderSize()) return; ReadChildren(atom_factory, stream, size-GetHeaderSize()); } From 26df396615a4b417fc887d041e18b8839555f061 Mon Sep 17 00:00:00 2001 From: Victor Loh Date: Mon, 1 Apr 2024 21:19:40 -0700 Subject: [PATCH 6/6] Add boundary checks for Ap4StsdAtom Same problem as before, underflow of bytes_available results in some potential attack --- Source/C++/Core/Ap4StsdAtom.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/C++/Core/Ap4StsdAtom.cpp b/Source/C++/Core/Ap4StsdAtom.cpp index e70972627..68f4a047c 100644 --- a/Source/C++/Core/Ap4StsdAtom.cpp +++ b/Source/C++/Core/Ap4StsdAtom.cpp @@ -87,6 +87,7 @@ AP4_StsdAtom::AP4_StsdAtom(AP4_UI32 size, AP4_AtomFactory& atom_factory) : AP4_ContainerAtom(AP4_ATOM_TYPE_STSD, size, false, version, flags) { + if (size < AP4_FULL_ATOM_HEADER_SIZE + 4) return; // read the number of entries AP4_UI32 entry_count; stream.ReadUI32(entry_count);