diff --git a/Cargo.lock b/Cargo.lock index 45648ba584..d56fe5399a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6329,6 +6329,7 @@ dependencies = [ "bcs", "codespan-reporting", "itertools 0.10.5", + "log", "move-binary-format", "move-command-line-common", "move-compiler", diff --git a/crates/rooch/src/commands/move_cli/commands/publish.rs b/crates/rooch/src/commands/move_cli/commands/publish.rs index 3992895f4e..935bfcc369 100644 --- a/crates/rooch/src/commands/move_cli/commands/publish.rs +++ b/crates/rooch/src/commands/move_cli/commands/publish.rs @@ -92,6 +92,7 @@ impl CommandAction for Publish { let sorted_modules = sort_by_dependency_order(modules.iter_modules())?; let resolver = context.get_client().await?; // Serialize and collect module binaries into bundles + verifier::verify_modules(&sorted_modules, &resolver)?; for module in sorted_modules { let module_address = module.self_id().address().to_owned(); if module_address != pkg_address { @@ -101,7 +102,6 @@ impl CommandAction for Publish { pkg_address.clone(), ))); }; - verifier::verify_module(&module, &resolver)?; let mut binary: Vec = vec![]; module.serialize(&mut binary)?; bundles.push(binary); diff --git a/frameworks/moveos-stdlib/src/natives/moveos_stdlib/move_module.rs b/frameworks/moveos-stdlib/src/natives/moveos_stdlib/move_module.rs index 102b175b7e..783d725378 100644 --- a/frameworks/moveos-stdlib/src/natives/moveos_stdlib/move_module.rs +++ b/frameworks/moveos-stdlib/src/natives/moveos_stdlib/move_module.rs @@ -175,13 +175,24 @@ fn native_sort_and_verify_modules_inner( let module_context = context.extensions_mut().get_mut::(); let mut module_ids = vec![]; let mut init_identifier = vec![]; + + let verify_result = + moveos_verifier::verifier::verify_modules(&compiled_modules, module_context.resolver); + match verify_result { + Ok(_) => {} + Err(e) => { + log::info!("modules verification error: {:?}", e); + return Ok(NativeResult::err(cost, E_MODULE_VERIFICATION_ERROR)); + } + } + for module in &compiled_modules { let module_address = *module.self_id().address(); if module_address != account_address { return Ok(NativeResult::err(cost, E_ADDRESS_NOT_MATCH_WITH_SIGNER)); } - let result = moveos_verifier::verifier::verify_module(module, module_context.resolver); + let result = moveos_verifier::verifier::verify_init_function(module); match result { Ok(res) => { if res { diff --git a/moveos/moveos-verifier/Cargo.toml b/moveos/moveos-verifier/Cargo.toml index 17acde3e85..20a4f84fed 100644 --- a/moveos/moveos-verifier/Cargo.toml +++ b/moveos/moveos-verifier/Cargo.toml @@ -24,4 +24,5 @@ move-vm-runtime = { workspace = true } move-vm-types = { workspace = true } move-symbol-pool = { workspace = true } -moveos-types = { workspace = true } \ No newline at end of file +moveos-types = { workspace = true } +log = "0.4.21" \ No newline at end of file diff --git a/moveos/moveos-verifier/src/build.rs b/moveos/moveos-verifier/src/build.rs index 7f27bc5acd..501f72baf0 100644 --- a/moveos/moveos-verifier/src/build.rs +++ b/moveos/moveos-verifier/src/build.rs @@ -341,6 +341,22 @@ pub fn inject_runtime_metadata>( CompiledUnit::Module(named_module) => { if let Some(module_metadata) = metadata.get(&named_module.module.self_id()) { if !module_metadata.is_empty() { + log::debug!( + "\n\nstart dump data structs map {:?}", + named_module.module.self_id().to_string() + ); + for (k, v) in module_metadata.data_struct_map.iter() { + log::debug!("{:?} -> {:?}", k, v); + } + log::debug!("\n"); + for (k, v) in module_metadata.data_struct_func_map.iter() { + log::debug!("{:?} -> {:?}", k, v); + } + log::debug!( + "start dump data structs map {:?}\n\n", + named_module.module.self_id().to_string() + ); + let serialized_metadata = bcs::to_bytes(&module_metadata).expect("BCS for RuntimeModuleMetadata"); named_module.module.metadata.push(Metadata { diff --git a/moveos/moveos-verifier/src/metadata.rs b/moveos/moveos-verifier/src/metadata.rs index 37ac0bb4ba..42280208dc 100644 --- a/moveos/moveos-verifier/src/metadata.rs +++ b/moveos/moveos-verifier/src/metadata.rs @@ -929,11 +929,16 @@ impl<'a> ExtendedChecker<'a> { impl<'a> ExtendedChecker<'a> { fn check_data_struct(&mut self, module_env: &ModuleEnv) { + let mut available_data_structs = BTreeMap::new(); + for struct_def in module_env.get_structs() { if is_data_struct_annotation(&struct_def, module_env) { if is_copy_drop_struct(&struct_def) { - let (error_message, is_allowed) = - check_data_struct_fields(&struct_def, module_env); + let (error_message, is_allowed) = check_data_struct_fields( + &struct_def, + module_env, + &mut available_data_structs, + ); if !is_allowed { self.env .error(&struct_def.get_loc(), error_message.as_str()); @@ -952,6 +957,20 @@ impl<'a> ExtendedChecker<'a> { } } + if !available_data_structs.is_empty() { + log::debug!( + "\n\ncheck_data_struct() module {:?} data structs start...", + module_env.get_full_name_str() + ); + for (k, v) in available_data_structs.iter() { + log::debug!("{:?} -> {:?}", k, v); + } + log::debug!( + "\n\ncheck_data_struct() module {:?} data structs end...", + module_env.get_full_name_str() + ); + } + let verified_module = match module_env.get_verified_module() { None => { self.env @@ -963,13 +982,10 @@ impl<'a> ExtendedChecker<'a> { let module_metadata = self.output.entry(verified_module.self_id()).or_default(); - let data_struct_map = unsafe { - let result: BTreeMap = GLOBAL_DATA_STRUCT - .iter() - .map(|(key, value)| (key.clone(), *value)) - .collect(); - result - }; + let data_struct_map: BTreeMap = available_data_structs + .iter() + .map(|(key, value)| (key.clone(), *value)) + .collect(); module_metadata.data_struct_map = data_struct_map; @@ -977,7 +993,11 @@ impl<'a> ExtendedChecker<'a> { } } -fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> (String, bool) { +fn check_data_struct_fields( + struct_def: &StructEnv, + module_env: &ModuleEnv, + valid_structs: &mut BTreeMap, +) -> (String, bool) { let struct_fields = struct_def.get_fields().collect_vec(); for field in struct_fields { let field_type = field.get_type(); @@ -985,7 +1005,7 @@ fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> ( .symbol_pool() .string(field.get_name()) .to_string(); - let is_allowed = check_data_struct_fields_type(&field_type, module_env); + let is_allowed = check_data_struct_fields_type(&field_type, module_env, valid_structs); if !is_allowed { let struct_name = module_env .symbol_pool() @@ -1008,6 +1028,7 @@ fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> ( .string(struct_def.get_name()) .to_string(); let full_struct_name = format!("{}::{}", module_env.get_full_name_str(), struct_name); + valid_structs.insert(full_struct_name.clone(), true); unsafe { GLOBAL_DATA_STRUCT.insert(full_struct_name, true); } @@ -1015,10 +1036,16 @@ fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> ( ("".to_string(), true) } -fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> bool { +fn check_data_struct_fields_type( + field_type: &Type, + module_env: &ModuleEnv, + valid_structs: &mut BTreeMap, +) -> bool { return match field_type { Type::Primitive(_) => true, - Type::Vector(item_type) => check_data_struct_fields_type(item_type.as_ref(), module_env), + Type::Vector(item_type) => { + check_data_struct_fields_type(item_type.as_ref(), module_env, valid_structs) + } Type::Struct(module_id, struct_id, ty_args) => { let module = module_env.env.get_module(*module_id); @@ -1037,7 +1064,7 @@ fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> b if is_std_option_type(&full_struct_name) { if let Some(item_type) = ty_args.first() { - return check_data_struct_fields_type(item_type, module_env); + return check_data_struct_fields_type(item_type, module_env, valid_structs); } return false; @@ -1051,7 +1078,7 @@ fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> b return false; } - check_data_struct_fields(&struct_env, &struct_module); + check_data_struct_fields(&struct_env, &struct_module, valid_structs); let is_allowed_opt = unsafe { GLOBAL_DATA_STRUCT.get(full_struct_name.as_str()) }; return if let Some(is_allowed) = is_allowed_opt { @@ -1248,13 +1275,24 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M .entry(compiled_module.self_id()) .or_default(); - let data_struct_func_map = unsafe { - let result: BTreeMap> = GLOBAL_DATA_STRUCT_FUNC - .iter() - .map(|(key, value)| (key.clone(), value.clone())) - .collect(); - result - }; + let data_struct_func_map: BTreeMap> = type_name_indices + .iter() + .map(|(key, value)| (key.clone(), value.clone())) + .collect(); + + if !data_struct_func_map.is_empty() { + log::debug!( + "\n\ncheck_data_struct_func() module {:?} data structs func start...", + module_env.get_full_name_str() + ); + for (k, v) in data_struct_func_map.iter() { + log::debug!("{:?} -> {:?}", k, v); + } + log::debug!( + "\n\ncheck_data_struct_func() module {:?} data structs func end...", + module_env.get_full_name_str() + ); + } module_metadata.data_struct_func_map = data_struct_func_map; @@ -1295,6 +1333,15 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M }; if let Some(data_struct_func_indicies) = data_struct_func_types { + let caller_fun_name = fun.get_full_name_str(); + log::debug!( + "function {:?}::{:?} call data_struct func {:?} with types {:?}", + module_env.self_address(), + caller_fun_name, + full_path_func_name, + data_struct_func_indicies + ); + for generic_type_index in data_struct_func_indicies { let type_arg = match type_arguments.get(generic_type_index) { None => { diff --git a/moveos/moveos-verifier/src/verifier.rs b/moveos/moveos-verifier/src/verifier.rs index 9eae24fba4..c07d0be20a 100644 --- a/moveos/moveos-verifier/src/verifier.rs +++ b/moveos/moveos-verifier/src/verifier.rs @@ -1,10 +1,9 @@ // Copyright (c) RoochNetwork // SPDX-License-Identifier: Apache-2.0 -use crate::metadata::{ - check_metadata_format, get_metadata_from_compiled_module, is_allowed_input_struct, - is_defined_or_allowed_in_current_module, -}; +use std::collections::BTreeMap; +use std::ops::Deref; + use move_binary_format::binary_views::BinaryIndexedView; use move_binary_format::errors::{Location, PartialVMError, PartialVMResult, VMError, VMResult}; use move_binary_format::file_format::{ @@ -22,20 +21,45 @@ use move_vm_runtime::data_cache::TransactionCache; use move_vm_runtime::session::{LoadedFunctionInstantiation, Session}; use move_vm_types::loaded_data::runtime_types::Type; use once_cell::sync::Lazy; -use std::ops::Deref; + +use crate::metadata::{ + check_metadata_format, get_metadata_from_compiled_module, is_allowed_input_struct, + is_defined_or_allowed_in_current_module, +}; pub static INIT_FN_NAME_IDENTIFIER: Lazy = Lazy::new(|| Identifier::new("init").unwrap()); -pub fn verify_module(module: &CompiledModule, db: Resolver) -> VMResult +pub fn verify_modules(modules: &Vec, db: Resolver) -> VMResult where Resolver: ModuleResolver, { - verify_private_generics(module, &db)?; + let mut verified_modules: BTreeMap = BTreeMap::new(); + for module in modules { + verify_private_generics(module, &db, &mut verified_modules)?; + verify_entry_function_at_publish(module)?; + verify_global_storage_access(module)?; + verify_gas_free_function(module)?; + verify_data_struct(module, &db, &mut verified_modules)?; + verify_init_function(module)?; + } + + Ok(true) +} + +pub fn verify_module( + module: &CompiledModule, + db: Resolver, + verified_modules: &mut BTreeMap, +) -> VMResult +where + Resolver: ModuleResolver, +{ + verify_private_generics(module, &db, verified_modules)?; verify_entry_function_at_publish(module)?; verify_global_storage_access(module)?; verify_gas_free_function(module)?; - verify_data_struct(module, &db)?; + verify_data_struct(module, &db, verified_modules)?; verify_init_function(module) } @@ -310,7 +334,11 @@ fn vm_error_for_init_func_checking( .finish(Location::Module(module_id)) } -pub fn verify_private_generics(module: &CompiledModule, db: &Resolver) -> VMResult +pub fn verify_private_generics( + module: &CompiledModule, + db: &Resolver, + verified_modules: &mut BTreeMap, +) -> VMResult where Resolver: ModuleResolver, { @@ -370,8 +398,13 @@ where if let Bytecode::CallGeneric(finst_idx) = instr { // Find the module where a function is located based on its InstantiationIndex, // and then find the metadata of the module. - let compiled_module_opt = - load_compiled_module_from_finst_idx(db, &view, finst_idx); + let compiled_module_opt = load_compiled_module_from_finst_idx( + db, + &view, + finst_idx, + verified_modules, + true, + ); if let Some(compiled_module) = compiled_module_opt { if let Err(err) = check_metadata_format(&compiled_module) { @@ -666,7 +699,11 @@ pub fn verify_gas_free_function(module: &CompiledModule) -> VMResult { Ok(true) } -pub fn verify_data_struct(caller_module: &CompiledModule, db: &Resolver) -> VMResult +pub fn verify_data_struct( + caller_module: &CompiledModule, + db: &Resolver, + verified_modules: &mut BTreeMap, +) -> VMResult where Resolver: ModuleResolver, { @@ -687,7 +724,7 @@ where } Some(metadata) => { - let data_structs_map = metadata.data_struct_map; + let mut data_structs_map = metadata.data_struct_map; let mut data_structs_func_map = metadata.data_struct_func_map; let view = BinaryIndexedView::Module(caller_module); @@ -697,8 +734,13 @@ where if let Bytecode::CallGeneric(finst_idx) = instr { // Find the module where a function is located based on its InstantiationIndex, // and then find the metadata of the module. - let compiled_module_opt = - load_compiled_module_from_finst_idx(db, &view, finst_idx); + let compiled_module_opt = load_compiled_module_from_finst_idx( + db, + &view, + finst_idx, + verified_modules, + true, + ); if let Some(callee_module) = compiled_module_opt { if let Err(err) = check_metadata_format(&callee_module) { @@ -763,18 +805,63 @@ where view.struct_handle_at(*struct_handle_idx); let module_handle = view.module_handle_at(struct_handle.module); - let full_struct_name = format!( - "{}::{}::{}", + let module_name = format!( + "{}::{}", view.address_identifier_at(module_handle.address) .to_hex_literal(), view.identifier_at(module_handle.name), + ); + + // load module from struct handle + let compiled_module_opt = + load_compiled_module_from_struct_handle( + db, + &view, + *struct_handle_idx, + verified_modules, + ); + if let Some(callee_module) = compiled_module_opt { + if let Err(err) = + check_metadata_format(&callee_module) + { + return Err(PartialVMError::new( + StatusCode::MALFORMED, + ) + .with_message(err.to_string()) + .finish(Location::Module( + callee_module.self_id(), + ))); + } + + // Find the definition records of compile-time data_struct from CompiledModule. + let metadata_opt = + get_metadata_from_compiled_module( + &callee_module, + ); + if let Some(metadata) = metadata_opt { + let _ = metadata + .data_struct_map + .iter() + .map(|(key, value)| { + data_structs_map + .insert(key.clone(), *value); + }) + .collect::>(); + } + } + + let full_struct_name = format!( + "{}::{}", + module_name, view.identifier_at(struct_handle.name) ); let is_data_struct_opt = data_structs_map.get(full_struct_name.as_str()); if is_data_struct_opt.is_none() { - let error_msg = format!("The type parameter {} when calling function {} is not a data_struct", - full_path_func_name, full_struct_name); + let caller_func_name = + build_full_function_name(&func.function, view); + let error_msg = format!("function {:} call {:} with type {:} is not a data struct.", + caller_func_name, full_path_func_name, full_struct_name); return generate_vm_error( StatusCode::TYPE_MISMATCH, error_msg, @@ -811,6 +898,7 @@ where } } + verified_modules.insert(caller_module.self_id(), caller_module.clone()); Ok(true) } @@ -941,11 +1029,34 @@ fn check_gas_charge_post_function( matches!(first_return_signature, SignatureToken::Bool) } +fn load_compiled_module_from_struct_handle( + db: &Resolver, + view: &BinaryIndexedView, + struct_idx: StructHandleIndex, + verified_modules: &mut BTreeMap, +) -> Option +where + Resolver: ModuleResolver, +{ + let struct_handle = view.struct_handle_at(struct_idx); + let module_handle = view.module_handle_at(struct_handle.module); + 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)); + + match verified_modules.get(&module_id) { + None => get_module_from_db(&module_id, db), + Some(m) => Some(m.clone()), + } +} + // Find the module where a function is located based on its InstantiationIndex. fn load_compiled_module_from_finst_idx( db: &Resolver, view: &BinaryIndexedView, finst_idx: FunctionInstantiationIndex, + verified_modules: &mut BTreeMap, + search_verified_modules: bool, ) -> Option where Resolver: ModuleResolver, @@ -961,13 +1072,25 @@ 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)); - match db.get_module(&module_id) { + if search_verified_modules { + match verified_modules.get(&module_id) { + None => get_module_from_db(&module_id, db), + Some(m) => Some(m.clone()), + } + } else { + get_module_from_db(&module_id, db) + } +} + +fn get_module_from_db(module_id: &ModuleId, db: &Resolver) -> Option +where + Resolver: ModuleResolver, +{ + match db.get_module(module_id) { Err(_) => None, Ok(value) => match value { None => None, - Some(bytes) => { - return CompiledModule::deserialize(bytes.as_slice()).ok(); - } + Some(bytes) => CompiledModule::deserialize(bytes.as_slice()).ok(), }, } } diff --git a/moveos/moveos/src/vm/moveos_vm.rs b/moveos/moveos/src/vm/moveos_vm.rs index 3eaf5ecda4..a4c522d426 100644 --- a/moveos/moveos/src/vm/moveos_vm.rs +++ b/moveos/moveos/src/vm/moveos_vm.rs @@ -203,7 +203,8 @@ where Err(err) => return Err(err.finish(Location::Undefined)), }; let script_module = script_into_module(compiled_script); - let result = moveos_verifier::verifier::verify_module(&script_module, self.remote); + let modules = vec![script_module]; + let result = moveos_verifier::verifier::verify_modules(&modules, self.remote); match result { Ok(_) => {} Err(err) => return Err(err), @@ -247,8 +248,16 @@ where )?; let mut init_function_modules = vec![]; + + let result = + moveos_verifier::verifier::verify_modules(&compiled_modules, self.remote); + match result { + Ok(_) => {} + Err(err) => return Err(err), + } + for module in &compiled_modules { - let result = moveos_verifier::verifier::verify_module(module, self.remote); + let result = moveos_verifier::verifier::verify_init_function(module); match result { Ok(res) => { if res {