From e46bf12d179cb6e50767d3c2fa6c3ffd621801c2 Mon Sep 17 00:00:00 2001
From: Dimitar Krastev <dimi1010100@gmail.com>
Date: Mon, 29 Jul 2024 04:57:37 +0300
Subject: [PATCH] Fix memory leak in PointerVector pushBack (#1487)

---
 Common++/header/PointerVector.h | 22 ++++++++++++++++++++--
 Packet++/header/Asn1Codec.h     |  2 +-
 Packet++/src/LdapLayer.cpp      |  2 +-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Common++/header/PointerVector.h b/Common++/header/PointerVector.h
index 96638da33f..299c190033 100644
--- a/Common++/header/PointerVector.h
+++ b/Common++/header/PointerVector.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <cstddef>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdexcept>
@@ -121,19 +122,36 @@ namespace pcpp
 			m_Vector.clear();
 		}
 
+		/**
+		 * Adding a nullptr to the vector is not allowed.
+		 */
+		void pushBack(std::nullptr_t element, bool freeElementOnError = true) = delete;
+
 		/**
 		 * Add a new (pointer to an) element to the vector
 		 * @param[in] element A pointer to an element to assume ownership of.
+		 * @param[in] freeElementOnError If set to true, the element is freed if an exception is thrown during the push.
 		 * @throws std::invalid_argument The provided pointer is a nullptr.
 		 */
-		void pushBack(T* element)
+		void pushBack(T* element, bool freeElementOnError = true)
 		{
 			if (element == nullptr)
 			{
 				throw std::invalid_argument("Element is nullptr");
 			}
 
-			m_Vector.push_back(element);
+			try
+			{
+				m_Vector.push_back(element);
+			}
+			catch (const std::exception&)
+			{
+				if (freeElementOnError)
+				{
+					delete element;
+				}
+				throw;
+			}
 		}
 
 		/**
diff --git a/Packet++/header/Asn1Codec.h b/Packet++/header/Asn1Codec.h
index 7eeb48a526..81ef57f1c2 100644
--- a/Packet++/header/Asn1Codec.h
+++ b/Packet++/header/Asn1Codec.h
@@ -321,7 +321,7 @@ namespace pcpp
 			{
 				auto encodedRecord = (*recordIter)->encode();
 				auto copyRecord = Asn1Record::decode(encodedRecord.data(), encodedRecord.size(), false);
-				m_SubRecords.pushBack(copyRecord.release());
+				m_SubRecords.pushBack(std::move(copyRecord));
 				recordValueLength += encodedRecord.size();
 			}
 
diff --git a/Packet++/src/LdapLayer.cpp b/Packet++/src/LdapLayer.cpp
index 5874c2a5e5..fd2f9bd10d 100644
--- a/Packet++/src/LdapLayer.cpp
+++ b/Packet++/src/LdapLayer.cpp
@@ -749,7 +749,7 @@ namespace pcpp {
 			Asn1OctetStringRecord typeRecord(attribute.type);
 			Asn1SetRecord valuesRecord(valuesSubRecords);
 
-			attributesSubRecords.pushBack(new Asn1SequenceRecord({&typeRecord, &valuesRecord}));
+			attributesSubRecords.pushBack(new Asn1SequenceRecord({ &typeRecord, &valuesRecord }));
 		}
 
 		Asn1OctetStringRecord objectNameRecord(objectName);