From e95a65499e5dc7befe1230de7fa17c97f330fb3c Mon Sep 17 00:00:00 2001 From: stevenlaw123 Date: Tue, 19 Dec 2023 21:38:42 +0800 Subject: [PATCH] Handle the unwrap function in the verifier. --- moveos/moveos-verifier/src/metadata.rs | 106 +++++++++++++++++++++---- moveos/moveos-verifier/src/verifier.rs | 76 ++++++++++++------ 2 files changed, 142 insertions(+), 40 deletions(-) diff --git a/moveos/moveos-verifier/src/metadata.rs b/moveos/moveos-verifier/src/metadata.rs index 06cca0899a..3010650165 100644 --- a/moveos/moveos-verifier/src/metadata.rs +++ b/moveos/moveos-verifier/src/metadata.rs @@ -156,7 +156,13 @@ impl<'a> ExtendedChecker<'a> { let mut type_name_indices: BTreeMap> = BTreeMap::new(); let mut func_loc_map = BTreeMap::new(); - let compiled_module = module.get_verified_module().unwrap(); + let compiled_module = match module.get_verified_module() { + None => { + return; + } + Some(module) => module, + }; + let view = BinaryIndexedView::Module(compiled_module); // Check every function and if a function has the private_generics attribute, @@ -272,6 +278,8 @@ impl<'a> ExtendedChecker<'a> { // verify that it calls a function with the private_generics attribute as detected earlier. // Then, ensure that the generic parameters of the CallGeneric instruction are valid. if fun.is_inline() { + // The fun.get_bytecode() will only be None when the function is an inline function. + // So we need to skip inline functions. continue; } for (offset, instr) in fun.get_bytecode().unwrap().iter().enumerate() { @@ -303,16 +311,29 @@ impl<'a> ExtendedChecker<'a> { }; if let Some(private_generics_types_indices) = private_generics_types { + let byte_loc = fun + .get_bytecode_loc(offset as u16) + .unwrap_or_else(|| Loc::default()); + for generic_type_index in private_generics_types_indices { - let type_arg = type_arguments.get(generic_type_index).unwrap(); + let type_arg = match type_arguments.get(generic_type_index) { + None => { + self.env.error( + &byte_loc, + format!( + "the function {:?} does not have enough type parameters.", full_path_func_name + ).as_str(), + ); + return; + } + Some(sig_token) => sig_token, + }; let (defined_in_current_module, struct_name) = is_defined_or_allowed_in_current_module(&view, type_arg); - let byte_loc = fun.get_bytecode_loc(offset as u16); - if !defined_in_current_module { self.env.error( - &byte_loc.unwrap(), + &byte_loc, format!( "resource type {:?} in function {:?} not defined in current module or not allowed", struct_name, full_path_func_name @@ -350,6 +371,8 @@ impl<'a> ExtendedChecker<'a> { fn check_init_module(&mut self, module: &ModuleEnv) { for ref fun in module.get_functions() { if fun.is_inline() { + // The fun.get_identifier() will only be None when the function is an inline function. + // So we need to skip inline functions. continue; } if fun.get_identifier().unwrap().as_ident_str() @@ -390,7 +413,8 @@ impl<'a> ExtendedChecker<'a> { self.env.error( &fun.get_loc(), "module init function should input a reference structure" - ) + ); + return; } if !check_storage_context_struct_tag(struct_tag.unwrap().to_canonical_string()){ @@ -525,6 +549,8 @@ impl<'a> ExtendedChecker<'a> { fn check_global_storage_access(&mut self, module: &ModuleEnv) { for ref fun in module.get_functions() { if fun.is_inline() { + // The fun.get_bytecode() will only be None when the function is an inline function. + // So we need to skip inline functions. continue; } let mut invalid_bytecode = vec![]; @@ -822,10 +848,16 @@ impl<'a> ExtendedChecker<'a> { module.symbol_pool().string(fenv.get_name())).as_str()); } - let module_metadata = self - .output - .entry(module.get_verified_module().unwrap().self_id()) - .or_default(); + let verified_module = match module.get_verified_module() { + None => { + self.env + .error(&fenv.get_loc(), "The verified module was not found."); + return; + } + Some(module) => module, + }; + + let module_metadata = self.output.entry(verified_module.self_id()).or_default(); module_metadata.gas_free_function_map = gas_free_function_map; } } @@ -857,10 +889,16 @@ impl<'a> ExtendedChecker<'a> { } } - let module_metadata = self - .output - .entry(module_env.get_verified_module().unwrap().self_id()) - .or_default(); + let verified_module = match module_env.get_verified_module() { + None => { + self.env + .error(&module_env.get_loc(), "The verified module was not found."); + return; + } + Some(module) => module, + }; + + let module_metadata = self.output.entry(verified_module.self_id()).or_default(); let data_struct_map = unsafe { let result: BTreeMap = GLOBAL_DATA_STRUCT @@ -989,7 +1027,16 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M let mut type_name_indices: BTreeMap> = BTreeMap::new(); let mut func_loc_map = BTreeMap::new(); - let compiled_module = module_env.get_verified_module().unwrap(); + let compiled_module = match module_env.get_verified_module() { + None => { + module_env + .env + .error(&module_env.get_loc(), "The verified module was not found."); + return; + } + Some(module) => module, + }; + let view = BinaryIndexedView::Module(compiled_module); for ref fun in module_env.get_functions() { @@ -1111,9 +1158,19 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M } } + let compiled_module = match module_env.get_verified_module() { + None => { + module_env + .env + .error(&module_env.get_loc(), "The verified module was not found."); + return; + } + Some(module) => module, + }; + let module_metadata = extended_checker .output - .entry(module_env.get_verified_module().unwrap().self_id()) + .entry(compiled_module.self_id()) .or_default(); let data_struct_func_map = unsafe { @@ -1128,6 +1185,8 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M for ref fun in module_env.get_functions() { if fun.is_inline() { + // The fun.get_bytecode() will only be None when the function is an inline function. + // So we need to skip inline functions. continue; } for (_offset, instr) in fun.get_bytecode().unwrap().iter().enumerate() { @@ -1162,7 +1221,13 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M if let Some(data_struct_func_indicies) = data_struct_func_types { for generic_type_index in data_struct_func_indicies { - let type_arg = type_arguments.get(generic_type_index).unwrap(); + let type_arg = match type_arguments.get(generic_type_index) { + None => { + extended_checker.env.error(&fun.get_loc(), ""); + return; + } + Some(v) => v, + }; let (is_allowed_struct_type, error_message) = check_func_data_struct(&view, fun, type_arg); @@ -1227,6 +1292,8 @@ fn check_func_data_struct( SignatureToken::U128 => (true, "".to_string()), SignatureToken::U256 => (true, "".to_string()), _ => { + // Only when the view is a Script, will view.self_id() return None. + // However, in our case, we are dealing with a CompiledModule, so it won't be None here. let module_id = view.self_id().unwrap().to_string(); (false, format!("The type argument of #[data_struct] for function {} in the module {} is not allowed.", func_name, module_id)) @@ -1262,6 +1329,7 @@ fn check_gas_validate_function(fenv: &FunctionEnv, global_env: &GlobalEnv) -> (b return (false, "return value length is less than 1".to_string()); } + // Length of the params_types array has already been checked above, so unwrap directly here. let storage_ctx_type = params_types.get(0).unwrap(); let parameter_checking_result = match storage_ctx_type { Type::Struct(module_id, struct_id, _) => { @@ -1291,6 +1359,7 @@ fn check_gas_validate_function(fenv: &FunctionEnv, global_env: &GlobalEnv) -> (b return parameter_checking_result; } + // Length of the return_types array has already been checked above, so unwrap directly here. let first_return_type = return_types.get(0).unwrap(); match first_return_type { Type::Primitive(PrimitiveType::Bool) => (true, "".to_string()), @@ -1313,6 +1382,7 @@ fn check_gas_charge_post_function(fenv: &FunctionEnv, global_env: &GlobalEnv) -> return (false, "Length of return values is less than 1.".to_string()); } + // Length of the params_types array has already been checked above, so unwrap directly here. let storage_ctx_type = params_types.get(0).unwrap(); match storage_ctx_type { Type::Struct(module_id, struct_id, _) => { @@ -1336,6 +1406,7 @@ fn check_gas_charge_post_function(fenv: &FunctionEnv, global_env: &GlobalEnv) -> ), } + // Length of the params_types array has already been checked above, so unwrap directly here. let gas_used_type = params_types.get(1).unwrap(); let second_parameter_checking_result = match *gas_used_type { Type::Primitive(PrimitiveType::U256) => (true, "".to_string()), @@ -1349,6 +1420,7 @@ fn check_gas_charge_post_function(fenv: &FunctionEnv, global_env: &GlobalEnv) -> return second_parameter_checking_result; } + // Length of the return_types array has already been checked above, so unwrap directly here. let first_return_type = return_types.get(0).unwrap(); match first_return_type { Type::Primitive(PrimitiveType::Bool) => (true, "".to_string()), diff --git a/moveos/moveos-verifier/src/verifier.rs b/moveos/moveos-verifier/src/verifier.rs index c0b647807b..ce1f4aef27 100644 --- a/moveos/moveos-verifier/src/verifier.rs +++ b/moveos/moveos-verifier/src/verifier.rs @@ -435,7 +435,18 @@ where if let Some(private_generics_types_indices) = private_generics_types { for generic_type_index in private_generics_types_indices { - let type_arg = type_arguments.get(*generic_type_index).unwrap(); + let type_arg = match type_arguments.get(*generic_type_index) { + None => { + return generate_vm_error( + StatusCode::RESOURCE_DOES_NOT_EXIST, + format!("the function {} does not have enough type arguments.", full_path_func_name), + None, + module, + ); + } + Some(v) => v, + }; + let (defined_in_current_module, struct_name) = is_defined_or_allowed_in_current_module(&view, type_arg); @@ -505,7 +516,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::RESOURCE_DOES_NOT_EXIST, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -525,7 +536,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::RESOURCE_DOES_NOT_EXIST, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -548,7 +559,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::RESOURCE_DOES_NOT_EXIST, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -564,7 +575,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::TOO_MANY_PARAMETERS, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -581,7 +592,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::TYPE_MISMATCH, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -602,7 +613,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::RESOURCE_DOES_NOT_EXIST, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -625,7 +636,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::RESOURCE_DOES_NOT_EXIST, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -641,7 +652,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::TOO_MANY_PARAMETERS, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -659,7 +670,7 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { return generate_vm_error( StatusCode::TYPE_MISMATCH, err_msg, - func_handle_index, + Some(func_handle_index), module, ); } @@ -749,7 +760,18 @@ where if let Some(data_struct_types_indices) = data_struct_func_types { for generic_type_index in data_struct_types_indices { - let type_arg = type_arguments.get(*generic_type_index).unwrap(); + let type_arg = match type_arguments.get(*generic_type_index) { + None => { + return generate_vm_error( + StatusCode::RESOURCE_DOES_NOT_EXIST, + format!("the function {} does not have enough type arguments.", full_path_func_name), + None, + caller_module, + ); + } + Some(v) => v, + }; + match type_arg { SignatureToken::Struct(struct_handle_idx) => { let struct_handle = @@ -770,7 +792,7 @@ where return generate_vm_error( StatusCode::TYPE_MISMATCH, error_msg, - *fhandle_idx, + Some(*fhandle_idx), caller_module, ); } @@ -781,7 +803,7 @@ where return generate_vm_error( StatusCode::TYPE_MISMATCH, error_msg, - *fhandle_idx, + Some(*fhandle_idx), caller_module, ); } @@ -815,12 +837,13 @@ fn generate_full_module_name( pub fn generate_vm_error( status_code: StatusCode, error_msg: String, - fhandle: FunctionHandleIndex, + fhandle: Option, module: &CompiledModule, ) -> VMResult { - Err(PartialVMError::new(status_code) - .with_message(error_msg) - .at_code_offset(FunctionDefinitionIndex::new(fhandle.0), 0_u16) + let err_incomplete = PartialVMError::new(status_code).with_message(error_msg); + let fdef_idx = fhandle.unwrap_or_else(|| FunctionHandleIndex::new(0)); + Err(err_incomplete + .at_code_offset(FunctionDefinitionIndex::new(fdef_idx.0), 0_u16) .finish(Location::Module(module.self_id()))) } @@ -849,7 +872,7 @@ fn check_gas_validate_function( func_signature: &Signature, return_signature: &Signature, ) -> bool { - // let mut parameter_check_result = false; + // Content of the func_signature array has already been checked above, so unwrap directly here. let first_parameter = func_signature.0.get(0).unwrap(); let check_struct_type = |struct_handle_idx: &StructHandleIndex| -> bool { @@ -878,6 +901,7 @@ fn check_gas_validate_function( return false; } + // Content of the return_signature array has already been checked above, so unwrap directly here. let first_return_signature = return_signature.0.get(0).unwrap(); matches!(first_return_signature, SignatureToken::Bool) } @@ -887,6 +911,7 @@ fn check_gas_charge_post_function( func_signature: &Signature, return_signature: &Signature, ) -> bool { + // Content of the func_signature array has already been checked above, so unwrap directly here. let first_parameter = func_signature.0.get(0).unwrap(); let check_struct_type = |struct_handle_idx: &StructHandleIndex| -> bool { @@ -913,6 +938,7 @@ fn check_gas_charge_post_function( return first_checking_result; } + // Content of the func_signature array has already been checked above, so unwrap directly here. let second_parameter = func_signature.0.get(1).unwrap(); let second_checking_result = matches!(second_parameter, SignatureToken::U128); @@ -924,6 +950,7 @@ fn check_gas_charge_post_function( return false; } + // Content of the return_signature array has already been checked above, so unwrap directly here. let first_return_signature = return_signature.0.get(0).unwrap(); matches!(first_return_signature, SignatureToken::Bool) } @@ -948,12 +975,15 @@ where let module_address = view.address_identifier_at(module_handle.address); let module_name = view.identifier_at(module_handle.name); let module_id = ModuleId::new(*module_address, Identifier::from(module_name)); - let module_bytes_opt = db.get_module(&module_id).unwrap(); - - if let Some(module_bytes) = module_bytes_opt { - return CompiledModule::deserialize(module_bytes.as_slice()).ok(); + match db.get_module(&module_id) { + Err(_) => None, + Ok(value) => match value { + None => None, + Some(bytes) => { + return CompiledModule::deserialize(bytes.as_slice()).ok(); + } + }, } - None } pub fn verify_global_storage_access(module: &CompiledModule) -> VMResult {