From 67604e1348599fac25193904f5b3622bd653b88f Mon Sep 17 00:00:00 2001 From: Peter LaFosse Date: Fri, 31 Oct 2025 11:19:15 -0400 Subject: [PATCH] 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 +++++++---- python/function.py | 24 ++++++++----- python/highlevelil.py | 4 +-- python/mediumlevelil.py | 3 +- python/types.py | 75 ++++++++++++++++++++++++----------------- 5 files changed, 77 insertions(+), 48 deletions(-) diff --git a/python/binaryview.py b/python/binaryview.py index a2b85c4b9..583c7e732 100644 --- a/python/binaryview.py +++ b/python/binaryview.py @@ -5286,8 +5286,8 @@ def define_data_var( 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): @@ -5321,8 +5321,11 @@ def define_user_data_var( 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): @@ -8377,6 +8380,7 @@ def define_type( 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) @@ -8407,6 +8411,7 @@ def define_user_type(self, name: Optional['_types.QualifiedNameType'], type_obj: 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']: @@ -8685,7 +8690,8 @@ def export_type_to_library(self, lib: typelibrary.TypeLibrary, name: Optional[st (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") @@ -8719,7 +8725,8 @@ def export_object_to_library( (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") diff --git a/python/function.py b/python/function.py index 0996d87fc..cf69cb3d5 100644 --- a/python/function.py +++ b/python/function.py @@ -1359,6 +1359,7 @@ def return_type(self, value: Optional[StringOrType]) -> None: # type: ignore type_conf.type = value.handle type_conf.confidence = core.max_confidence else: + value = value.immutable_copy() type_conf.type = value.handle type_conf.confidence = value.confidence core.BNSetUserFunctionReturnType(self.handle, type_conf) @@ -2270,11 +2271,14 @@ def create_graph_immediate( def apply_imported_types(self, sym: 'types.CoreSymbol', type: Optional[StringOrType] = None) -> None: if isinstance(type, str): (type, _) = self.view.parse_type_string(type) + if type is not None: + type = type.immutable_copy() core.BNApplyImportedTypes(self.handle, sym.handle, None if type is None else type.handle) def apply_auto_discovered_type(self, func_type: StringOrType) -> None: if isinstance(func_type, str): (func_type, _) = self.view.parse_type_string(func_type) + func_type = func_type.immutable_copy() core.BNApplyAutoDiscoveredFunctionType(self.handle, func_type.handle) def set_auto_indirect_branches( @@ -2440,11 +2444,13 @@ def get_block_annotations(self, addr: int, def set_auto_type(self, value: StringOrType) -> None: if isinstance(value, str): (value, _) = self.view.parse_type_string(value) + value = value.immutable_copy() core.BNSetFunctionAutoType(self.handle, value.handle) def set_user_type(self, value: StringOrType) -> None: if isinstance(value, str): (value, _) = self.view.parse_type_string(value) + value = value.immutable_copy() core.BNSetFunctionUserType(self.handle, value.handle) @property @@ -2462,6 +2468,7 @@ def set_auto_return_type(self, value: StringOrType) -> None: type_conf.type = value type_conf.confidence = core.max_confidence else: + value = value.immutable_copy() type_conf.type = value.handle type_conf.confidence = value.confidence core.BNSetAutoFunctionReturnType(self.handle, type_conf) @@ -2807,14 +2814,14 @@ def set_user_instr_highlight( def create_auto_stack_var(self, offset: int, var_type: StringOrType, name: str) -> None: if isinstance(var_type, str): (var_type, _) = self.view.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNCreateAutoStackVariable(self.handle, offset, tc, name) + tc = var_type.immutable_copy() + core.BNCreateAutoStackVariable(self.handle, offset, tc._to_core_struct(), name) def create_user_stack_var(self, offset: int, var_type: StringOrType, name: str) -> None: if isinstance(var_type, str): (var_type, _) = self.view.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNCreateUserStackVariable(self.handle, offset, tc, name) + tc = var_type.immutable_copy() + core.BNCreateUserStackVariable(self.handle, offset, tc._to_core_struct(), name) def delete_auto_stack_var(self, offset: int) -> None: core.BNDeleteAutoStackVariable(self.handle, offset) @@ -2827,16 +2834,16 @@ def create_auto_var( ) -> None: if isinstance(var_type, str): (var_type, _) = self.view.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNCreateAutoVariable(self.handle, var.to_BNVariable(), tc, name, ignore_disjoint_uses) + tc = var_type.immutable_copy() + core.BNCreateAutoVariable(self.handle, var.to_BNVariable(), tc._to_core_struct(), name, ignore_disjoint_uses) def create_user_var( self, var: 'variable.Variable', var_type: StringOrType, name: str, ignore_disjoint_uses: bool = False ) -> None: if isinstance(var_type, str): (var_type, _) = self.view.parse_type_string(var_type) - tc = var_type._to_core_struct() - core.BNCreateUserVariable(self.handle, var.to_BNVariable(), tc, name, ignore_disjoint_uses) + tc = var_type.immutable_copy() + core.BNCreateUserVariable(self.handle, var.to_BNVariable(), tc._to_core_struct(), name, ignore_disjoint_uses) def delete_user_var(self, var: 'variable.Variable') -> None: core.BNDeleteUserVariable(self.handle, var.to_BNVariable()) @@ -2955,6 +2962,7 @@ def set_call_type_adjustment( else: confidence = adjust_type.confidence type_conf = core.BNTypeWithConfidence() + adjust_type = adjust_type.immutable_copy() type_conf.type = adjust_type.handle type_conf.confidence = confidence else: diff --git a/python/highlevelil.py b/python/highlevelil.py index ef1b3de2a..71865d224 100644 --- a/python/highlevelil.py +++ b/python/highlevelil.py @@ -4816,8 +4816,8 @@ def set_expr_type(self, expr_index: int, expr_type: StringOrType) -> None: """ if isinstance(expr_type, str): (expr_type, _) = self.view.parse_type_string(expr_type) - tc = expr_type._to_core_struct() - core.BNSetHighLevelILExprType(self.handle, expr_index, tc) + ic = expr_type.immutable_copy() + core.BNSetHighLevelILExprType(self.handle, expr_index, ic._to_core_struct()) class HighLevelILBasicBlock(basicblock.BasicBlock): diff --git a/python/mediumlevelil.py b/python/mediumlevelil.py index e710d5c27..638dfbc60 100644 --- a/python/mediumlevelil.py +++ b/python/mediumlevelil.py @@ -6311,7 +6311,8 @@ def set_expr_type(self, expr_index: int, expr_type: Optional[StringOrType]) -> N if expr_type is not None: if isinstance(expr_type, str): (expr_type, _) = self.view.parse_type_string(expr_type) - tc = expr_type._to_core_struct() + ic = expr_type.immutable_copy() + tc = ic._to_core_struct() else: tc = core.BNTypeWithConfidence() tc.type = None diff --git a/python/types.py b/python/types.py index 5b3de7c9f..398e241dc 100644 --- a/python/types.py +++ b/python/types.py @@ -435,9 +435,10 @@ class FunctionParameter: location: Optional['variable.VariableNameAndType'] = None def __repr__(self): + ic = self.type.immutable_copy() if (self.location is not None) and (self.location.name != self.name): - return f"{self.type.immutable_copy().get_string_before_name()} {self.name}{self.type.immutable_copy().get_string_after_name()} @ {self.location.name}" - return f"{self.type.immutable_copy().get_string_before_name()} {self.name}{self.type.immutable_copy().get_string_after_name()}" + return f"{ic.get_string_before_name()} {self.name}{ic.get_string_after_name()} @ {self.location.name}" + return f"{ic.get_string_before_name()} {self.name}{ic.get_string_after_name()}" def immutable_copy(self) -> 'FunctionParameter': return FunctionParameter(self.type.immutable_copy(), self.name, self.location) @@ -602,18 +603,8 @@ def __repr__(self): def __str__(self): return str(self.immutable_copy()) - @property - def handle(self) -> core.BNTypeHandle: - return self.immutable_copy().handle - def __hash__(self): - return hash(ctypes.addressof(self.handle.contents)) - - def _to_core_struct(self) -> core.BNTypeWithConfidence: - type_conf = core.BNTypeWithConfidence() - type_conf.type = self.handle - type_conf.confidence = self.confidence - return type_conf + return hash(ctypes.addressof(self._handle.contents)) def immutable_copy(self): Types = { @@ -832,7 +823,8 @@ def child(self) -> 'Type': @child.setter def child(self, value: SomeType) -> None: - core.BNTypeBuilderSetChildType(self._handle, value.immutable_copy()._to_core_struct()) + ic = value.immutable_copy() + core.BNTypeBuilderSetChildType(self._handle, ic._to_core_struct()) @property def alternate_name(self) -> Optional[str]: @@ -978,7 +970,8 @@ def create( _const = BoolWithConfidence.get_core_struct(const) _volatile = BoolWithConfidence.get_core_struct(volatile) - handle = core.BNCreatePointerTypeBuilderOfWidth(_width, type._to_core_struct(), _const, _volatile, ref_type) + ic = type.immutable_copy() + handle = core.BNCreatePointerTypeBuilderOfWidth(_width, ic._to_core_struct(), _const, _volatile, ref_type) assert handle is not None, "BNCreatePointerTypeBuilderOfWidth returned None" return cls(handle, platform, confidence) @@ -1099,7 +1092,8 @@ def create( cls, type: SomeType, element_count: int, platform: Optional['_platform.Platform'] = None, confidence: int = core.max_confidence ) -> 'ArrayBuilder': - handle = core.BNCreateArrayTypeBuilder(type._to_core_struct(), element_count) + ic = type.immutable_copy() + handle = core.BNCreateArrayTypeBuilder(ic._to_core_struct(), element_count) assert handle is not None, "BNCreateArrayTypeBuilder returned None" return cls(handle, platform, confidence) @@ -1127,11 +1121,11 @@ def create( name_type: 'NameType' = NameType.NoNameType, pure: Optional[BoolWithConfidence] = None ) -> 'FunctionBuilder': - param_buf = FunctionBuilder._to_core_struct(params) + param_buf, type_list = FunctionBuilder._to_core_struct(params) if return_type is None: - ret_conf = Type.void()._to_core_struct() + ret_conf = Type.void() else: - ret_conf = return_type._to_core_struct() + ret_conf = return_type.immutable_copy() conv_conf = core.BNCallingConventionWithConfidence() if calling_convention is None: @@ -1185,7 +1179,7 @@ def create( if params is None: params = [] handle = core.BNCreateFunctionTypeBuilder( - ret_conf, conv_conf, param_buf, len(params), vararg_conf, can_return_conf, stack_adjust_conf, + ret_conf._to_core_struct(), conv_conf, param_buf, len(params), vararg_conf, can_return_conf, stack_adjust_conf, reg_stack_adjust_regs, reg_stack_adjust_values, len(reg_stack_adjust), return_regs_set, name_type, pure_conf ) @@ -1282,10 +1276,18 @@ def variable_arguments(self) -> BoolWithConfidence: def _to_core_struct(params: Optional[ParamsType] = None): if params is None: params = [] + + # type_list is very important as we need to keep a reference to the intermediate type + # objects as we're getting their handles if they go out of scope while we're holding + # their handles we get a UAF. This is only necessary as we're inside a helper that + # has to deal with raw type objects + type_list = [] param_buf = (core.BNFunctionParameter * len(params))() for i, param in enumerate(params): core_param = param_buf[i] if isinstance(param, (Type, TypeBuilder)): + param = param.immutable_copy() + type_list.append(param) assert param.handle is not None, "Attempting to construct function parameter without properly constructed type" core_param.name = "" core_param.type = param.handle @@ -1293,9 +1295,11 @@ def _to_core_struct(params: Optional[ParamsType] = None): core_param.defaultLocation = True elif isinstance(param, FunctionParameter): assert param.type is not None, "Attempting to construct function parameter without properly constructed type" + param_type = param.type.immutable_copy() + type_list.append(param_type) core_param.name = param.name - core_param.type = param.type.handle - core_param.typeConfidence = param.type.confidence + core_param.type = param_type.handle + core_param.typeConfidence = param_type.confidence if param.location is None: core_param.defaultLocation = True else: @@ -1307,13 +1311,15 @@ def _to_core_struct(params: Optional[ParamsType] = None): name, _type = param if not isinstance(name, str) or not isinstance(_type, (Type, TypeBuilder)): raise ValueError(f"Conversion from unsupported function parameter type {type(param)}") + _type = _type.immutable_copy() + type_list.append(_type) core_param.name = name core_param.type = _type.handle core_param.typeConfidence = _type.confidence core_param.defaultLocation = True else: raise ValueError(f"Conversion from unsupported function parameter type {type(param)}") - return param_buf + return param_buf, type_list @parameters.setter def parameters(self, params: List[FunctionParameter]) -> None: @@ -1438,17 +1444,20 @@ def _add_members_to_builder(structure_builder_handle, members: Optional[MembersT for member in members: if isinstance(member, Tuple): _type, _name = member + ic = _type.immutable_copy() core.BNAddStructureBuilderMember( - structure_builder_handle, _type._to_core_struct(), _name, MemberAccess.NoAccess, MemberScope.NoScope + structure_builder_handle, ic._to_core_struct(), _name, MemberAccess.NoAccess, MemberScope.NoScope ) elif isinstance(member, StructureMember): + ic = member.type.immutable_copy() core.BNAddStructureBuilderMemberAtOffset( - structure_builder_handle, member.type._to_core_struct(), member.name, member.offset, False, + structure_builder_handle, ic._to_core_struct(), member.name, member.offset, False, member.access, member.scope, member.bit_position, member.bit_width ) elif isinstance(member, (TypeBuilder, Type)): + ic = member.immutable_copy() core.BNAddStructureBuilderMember( - structure_builder_handle, member._to_core_struct(), "", MemberAccess.NoAccess, MemberScope.NoScope + structure_builder_handle, ic._to_core_struct(), "", MemberAccess.NoAccess, MemberScope.NoScope ) else: raise ValueError(f"Structure member type {member} not supported") @@ -1611,8 +1620,9 @@ def index_by_offset(self, offset: MemberOffset) -> Optional[MemberIndex]: return None def replace(self, index: int, type: SomeType, name: str = "", overwrite_existing: bool = True): + ic = type.immutable_copy() core.BNReplaceStructureBuilderMember( - self.builder_handle, index, type._to_core_struct(), name, overwrite_existing + self.builder_handle, index, ic._to_core_struct(), name, overwrite_existing ) def remove(self, index: int): @@ -1622,8 +1632,9 @@ def insert( self, offset: int, type: SomeType, name: str = "", overwrite_existing: bool = True, access: MemberAccess = MemberAccess.NoAccess, scope: MemberScope = MemberScope.NoScope, bit_position: int = 0, bit_width: int = 0 ): + ic = type.immutable_copy() core.BNAddStructureBuilderMemberAtOffset( - self.builder_handle, type._to_core_struct(), name, offset, overwrite_existing, access, scope, bit_position, + self.builder_handle, ic._to_core_struct(), name, offset, overwrite_existing, access, scope, bit_position, bit_width ) @@ -1632,7 +1643,8 @@ def append( scope: MemberScope = MemberScope.NoScope ) -> 'StructureBuilder': # appends a member at the end of the structure growing the structure - core.BNAddStructureBuilderMember(self.builder_handle, type._to_core_struct(), name, access, scope) + ic = type.immutable_copy() + core.BNAddStructureBuilderMember(self.builder_handle, ic._to_core_struct(), name, access, scope) return self def add_member_at_offset( @@ -1640,8 +1652,9 @@ def add_member_at_offset( access: MemberAccess = MemberAccess.NoAccess, scope: MemberScope = MemberScope.NoScope, bit_position: int = 0, bit_width: int = 0 ) -> 'StructureBuilder': # Adds structure member to the given offset optionally clearing any members within the range offset-offset+len(type) + ic = type.immutable_copy() core.BNAddStructureBuilderMemberAtOffset( - self.builder_handle, type._to_core_struct(), name, offset, overwrite_existing, access, scope, bit_position, + self.builder_handle, ic._to_core_struct(), name, offset, overwrite_existing, access, scope, bit_position, bit_width ) return self @@ -3083,7 +3096,7 @@ def create( ret = VoidType.create() if params is None: params = [] - param_buf = FunctionBuilder._to_core_struct(params) + param_buf, type_list = FunctionBuilder._to_core_struct(params) ret_conf = ret._to_core_struct() conv_conf = core.BNCallingConventionWithConfidence() if calling_convention is None: