Fix leaking BNDataVariableAndName when calling BNGetDebugDataVariableBy functions

Author: emesare

binaryninjacore.h

@@ -37,14 +37,14 @@
 // Current ABI version for linking to the core. This is incremented any time
 // there are changes to the API that affect linking, including new functions,
 // new types, or modifications to existing functions or types.
-#define BN_CURRENT_CORE_ABI_VERSION 93
+#define BN_CURRENT_CORE_ABI_VERSION 94
 
 // Minimum ABI version that is supported for loading of plugins. Plugins that
 // are linked to an ABI version less than this will not be able to load and
 // will require rebuilding. The minimum version is increased when there are
 // incompatible changes that break binary compatibility, such as changes to
 // existing types or functions.
-#define BN_MINIMUM_CORE_ABI_VERSION 92
+#define BN_MINIMUM_CORE_ABI_VERSION 94
 
 #ifdef __GNUC__
 	#ifdef BINARYNINJACORE_LIBRARY
@@ -7555,10 +7555,10 @@ extern "C"
 		BNDebugInfo* const debugInfo, const BNDataVariableAndName* var);
 	BINARYNINJACOREAPI BNDataVariableAndName* BNGetDebugDataVariables(
 		BNDebugInfo* const debugInfo, const char* const name, size_t* count);
-	BINARYNINJACOREAPI BNDataVariableAndName* BNGetDebugDataVariableByName(
-		BNDebugInfo* const debugInfo, const char* const parserName, const char* const variableName);
-	BINARYNINJACOREAPI BNDataVariableAndName* BNGetDebugDataVariableByAddress(
-		BNDebugInfo* const debugInfo, const char* const parserName, const uint64_t address);
+	BINARYNINJACOREAPI bool BNGetDebugDataVariableByName(
+		BNDebugInfo* const debugInfo, const char* const parserName, const char* const variableName, BNDataVariableAndName* var);
+	BINARYNINJACOREAPI bool BNGetDebugDataVariableByAddress(
+		BNDebugInfo* const debugInfo, const char* const parserName, const uint64_t address, BNDataVariableAndName* var);
 	BINARYNINJACOREAPI BNDataVariableAndName* BNGetDebugDataVariablesByName(
 		BNDebugInfo* const debugInfo, const char* const variableName, size_t* count);
 	BINARYNINJACOREAPI BNDataVariableAndNameAndDebugParser* BNGetDebugDataVariablesByAddress(

debuginfo.cpp

@@ -158,27 +158,25 @@ Ref<Type> DebugInfo::GetTypeByName(const string& parserName, const string& name)
 optional<tuple<uint64_t, Ref<Type>>> DebugInfo::GetDataVariableByName(
 	const string& parserName, const string& name) const
 {
-	BNDataVariableAndName* result = BNGetDebugDataVariableByName(m_object, parserName.c_str(), name.c_str());
-	if (result)
-	{
-		BNFreeString(result->name);
-		return {{result->address, Ref<Type>(new Type(result->type))}};
-	}
-	return {};
+	BNDataVariableAndName result;
+	if (!BNGetDebugDataVariableByName(m_object, parserName.c_str(), name.c_str(), &result))
+		return std::nullopt;
+	Ref<Type> type = new Type(BNNewTypeReference(result.type));
+	BNFreeDataVariableAndName(&result);
+	return {{result.address, type}};
 }
 
 
 optional<tuple<string, Ref<Type>>> DebugInfo::GetDataVariableByAddress(
 	const string& parserName, const uint64_t address) const
 {
-	BNDataVariableAndName* nameAndVar = BNGetDebugDataVariableByAddress(m_object, parserName.c_str(), address);
-	if (nameAndVar)
-	{
-		const tuple<string, Ref<Type>> result = {nameAndVar->name, Ref<Type>(new Type(nameAndVar->type))};
-		BNFreeString(nameAndVar->name);
-		return {result};
-	}
-	return {};
+	BNDataVariableAndName result;
+	if (!BNGetDebugDataVariableByAddress(m_object, parserName.c_str(), address, &result))
+		return std::nullopt;
+	string name = result.name;
+	Ref<Type> type = new Type(BNNewTypeReference(result.type));
+	BNFreeDataVariableAndName(&result);
+	return {{name, type}};
 }
 
 

python/debuginfo.py

@@ -399,19 +399,20 @@ def get_type_by_name(self, parser_name: str, name: str) -> Optional[_types.Type]
 		return None
 
 	def get_data_variable_by_name(self, parser_name: str, name: str) -> Optional[Tuple[int, _types.Type]]:
-		result = core.BNGetDebugDataVariableByName(self.handle, parser_name, name)
-		if result is not None:
-			core.BNFreeString(result.name)
-			return (result.address, _types.Type.create(result.type))
-		return None
+		name_and_var = core.BNDataVariableAndName()
+		if not core.BNGetDebugDataVariableByName(self.handle, parser_name, name, name_and_var):
+			return None
+		result = (name_and_var.address, _types.Type.create(core.BNNewTypeReference(name_and_var.type)))
+		core.BNFreeDataVariableAndName(name_and_var)
+		return result
 
 	def get_data_variable_by_address(self, parser_name: str, address: int) -> Optional[Tuple[str, _types.Type]]:
-		name_and_var = core.BNGetDebugDataVariableByAddress(self.handle, parser_name, address)
-		if name_and_var is not None:
-			result = (str(name_and_var.name), _types.Type.create(name_and_var.type))
-			core.BNFreeString(name_and_var.name)
-			return result
-		return None
+		name_and_var = core.BNDataVariableAndName()
+		if not core.BNGetDebugDataVariableByAddress(self.handle, parser_name, address, name_and_var):
+			return None
+		result = (str(name_and_var.name), _types.Type.create(core.BNNewTypeReference(name_and_var.type)))
+		core.BNFreeDataVariableAndName(name_and_var)
+		return result
 
 	def get_types_by_name(self, name: str) -> List[Tuple[str, _types.Type]]:
 		""" The first element in the Tuple returned in the list is the name of the debug info parser the type came from """

rust/src/debuginfo.rs

@@ -562,18 +562,18 @@ impl DebugInfo {
     ) -> Option<NamedDataVariableWithType> {
         let parser_name = parser_name.into_bytes_with_nul();
         let name = name.into_bytes_with_nul();
-        let raw_named_var = unsafe {
-            BNGetDebugDataVariableByName(
+        let mut dv = BNDataVariableAndName::default();
+        unsafe {
+            if BNGetDebugDataVariableByName(
                 self.handle,
                 parser_name.as_ref().as_ptr() as *mut _,
                 name.as_ref().as_ptr() as *mut _,
-            )
-        };
-
-        if !raw_named_var.is_null() {
-            Some(unsafe { NamedDataVariableWithType::from_ref_raw(raw_named_var) })
-        } else {
-            None
+                &mut dv,
+            ) {
+                Some(NamedDataVariableWithType::from_owned_raw(dv))
+            } else {
+                None
+            }
         }
     }
 
@@ -583,18 +583,18 @@ impl DebugInfo {
         address: u64,
     ) -> Option<NamedDataVariableWithType> {
         let parser_name = parser_name.into_bytes_with_nul();
-        let raw_named_var = unsafe {
-            BNGetDebugDataVariableByAddress(
+        let mut dv = BNDataVariableAndName::default();
+        unsafe {
+            if BNGetDebugDataVariableByAddress(
                 self.handle,
                 parser_name.as_ref().as_ptr() as *mut _,
                 address,
-            )
-        };
-
-        if !raw_named_var.is_null() {
-            Some(unsafe { NamedDataVariableWithType::from_ref_raw(raw_named_var) })
-        } else {
-            None
+                &mut dv,
+            ) {
+                Some(NamedDataVariableWithType::from_owned_raw(dv))
+            } else {
+                None
+            }
         }
     }
 

rust/src/variable.rs

@@ -119,12 +119,6 @@ impl NamedDataVariableWithType {
         owned
     }
 
-    pub(crate) unsafe fn from_ref_raw(value: *mut BNDataVariableAndName) -> Self {
-        let owned = Self::from_raw(&*value);
-        Self::free_ref_raw(value);
-        owned
-    }
-
     pub(crate) fn into_raw(value: Self) -> BNDataVariableAndName {
         let bn_name = BnString::new(value.name);
         BNDataVariableAndName {
@@ -136,10 +130,6 @@ impl NamedDataVariableWithType {
         }
     }
 
-    pub(crate) fn free_ref_raw(value: *mut BNDataVariableAndName) {
-        unsafe { BNFreeDataVariableAndName(value) }
-    }
-
     pub(crate) fn free_raw(value: BNDataVariableAndName) {
         let _ = unsafe { Type::ref_from_raw(value.type_) };
         let _ = unsafe { BnString::from_raw(value.name) };