From 2099579041340399246976046395863741933136 Mon Sep 17 00:00:00 2001 From: Peter LaFosse Date: Fri, 31 Oct 2025 11:19:15 -0400 Subject: Fix a fundamental problem with TypeBuilder.handle by deleting it TypeBuilder.handle creates an immutable_type gets the handle and then deletes the object to which the handle belonged to. This was fundamentally a UAF. immutable_copy is an error prone api as its very easy to misuse. We should consider a re-architecture of this API. `TypeBuilder.immutable_copy().` or `Type.mutable_copy().` should be considered sketchy and immutable_copy().handle is just a straight up UAF. Prior to this commit the following would cause a crash: current_data_variable.type = PointerBuilder.create(FunctionType.create()) --- python/binaryview.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'python/binaryview.py') diff --git a/python/binaryview.py b/python/binaryview.py index 54898478..2f441f34 100644 --- a/python/binaryview.py +++ b/python/binaryview.py @@ -5432,8 +5432,8 @@ class BinaryView: if isinstance(var_type, str): (var_type, _) = self.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNDefineDataVariable(self.handle, addr, tc) + tc = var_type.immutable_copy() + core.BNDefineDataVariable(self.handle, addr, tc._to_core_struct()) if name is not None: if isinstance(name, str): @@ -5467,8 +5467,11 @@ class BinaryView: if isinstance(var_type, str): (var_type, _) = self.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNDefineUserDataVariable(self.handle, addr, tc) + + # this var_type temporary is essential! It holds the reference to the immutable type + # until after BNDefineUserDataVariable takes the reference. + var_type = var_type.immutable_copy() + core.BNDefineUserDataVariable(self.handle, addr, var_type._to_core_struct()) if name is not None: if isinstance(name, str): @@ -8523,6 +8526,7 @@ class BinaryView: default_name = new_name assert default_name is not None, "default_name can only be None if named type is derived from string passed to type_obj" name = _types.QualifiedName(default_name)._to_core_struct() + type_obj = type_obj.immutable_copy() reg_name = core.BNDefineAnalysisType(self.handle, type_id, name, type_obj.handle) result = _types.QualifiedName._from_core_struct(reg_name) core.BNFreeQualifiedName(reg_name) @@ -8553,6 +8557,7 @@ class BinaryView: if name is None: raise ValueError("name can only be None if named type is derived from string passed to type_obj") _name = _types.QualifiedName(name)._to_core_struct() + type_obj = type_obj.immutable_copy() core.BNDefineUserAnalysisType(self.handle, _name, type_obj.handle) def define_types(self, types: List[Tuple[str, Optional['_types.QualifiedNameType'], StringOrType]], progress_func: Optional[ProgressFuncType]) -> Mapping[str, '_types.QualifiedName']: @@ -8831,7 +8836,8 @@ class BinaryView: (type_obj, new_name) = self.parse_type_string(type_obj) if name is None: _name = new_name - if not isinstance(type_obj, (_types.Type, _types.TypeBuilder)): + type_obj = type_obj.immutable_copy() + if not isinstance(type_obj, _types.Type): raise TypeError("type_obj must be a Type object") if _name is None: raise ValueError("name can only be None if named type is derived from string passed to type_obj") @@ -8865,7 +8871,8 @@ class BinaryView: (type_obj, new_name) = self.parse_type_string(type_obj) if name is None: _name = new_name - if not isinstance(type_obj, (_types.Type, _types.TypeBuilder)): + type_obj = type_obj.immutable_copy() + if not isinstance(type_obj, _types.Type): raise TypeError("type_obj must be a Type object") if _name is None: raise ValueError("name can only be None if named type is derived from string passed to type_obj") -- cgit v1.3.1