diff --git a/sui/src/sui-move.rs b/sui/src/sui-move.rs index d00df25273fe6..e11e28521ce93 100644 --- a/sui/src/sui-move.rs +++ b/sui/src/sui-move.rs @@ -20,9 +20,10 @@ pub enum MoveCommands { impl MoveCommands { pub fn execute(&self, path: &Path) -> Result<(), anyhow::Error> { + // TODO: Make dev_mode configurable. match self { Self::Build => { - sui_framework::build_and_verify_user_package(path, false)?; + sui_framework::build_and_verify_user_package(path, true)?; } Self::Test => { sui_framework::build_and_verify_user_package(path, true).unwrap(); diff --git a/sui_programmability/adapter/src/adapter.rs b/sui_programmability/adapter/src/adapter.rs index 556603d5345b9..4fae2f923cab2 100644 --- a/sui_programmability/adapter/src/adapter.rs +++ b/sui_programmability/adapter/src/adapter.rs @@ -451,7 +451,7 @@ pub fn verify_and_link< // run the Sui verifier for module in modules.iter() { // Run Sui bytecode verifier, which runs some additional checks that assume the Move bytecode verifier has passed. - verifier::verify_module(module)?; + verifier::verify_module(module, verifier::VerifyFlag::ForPublish)?; } Ok(vm) } diff --git a/sui_programmability/framework/deps/move-stdlib/sources/Debug.move b/sui_programmability/framework/deps/move-stdlib/sources/Debug.move new file mode 100644 index 0000000000000..777a75d035749 --- /dev/null +++ b/sui_programmability/framework/deps/move-stdlib/sources/Debug.move @@ -0,0 +1,6 @@ +/// Module providing debug functionality. +module Std::Debug { + native public fun print(x: &T); + + native public fun print_stack_trace(); +} diff --git a/sui_programmability/framework/src/lib.rs b/sui_programmability/framework/src/lib.rs index 62eb8625d9bea..7435af06065b8 100644 --- a/sui_programmability/framework/src/lib.rs +++ b/sui_programmability/framework/src/lib.rs @@ -7,6 +7,7 @@ use move_package::BuildConfig; use num_enum::TryFromPrimitive; use std::collections::HashSet; use std::path::Path; +use sui_bytecode_verifier::VerifyFlag; use sui_types::error::{SuiError, SuiResult}; use sui_verifier::verifier as sui_bytecode_verifier; @@ -40,7 +41,7 @@ pub enum EventType { pub fn get_sui_framework_modules(lib_dir: &Path) -> SuiResult> { let modules = build_framework(lib_dir)?; - verify_modules(&modules)?; + verify_modules(&modules, VerifyFlag::ForPublish)?; Ok(modules) } @@ -54,7 +55,7 @@ pub fn get_move_stdlib_modules(lib_dir: &Path) -> SuiResult> .into_iter() .filter(|m| !denylist.contains(&m.self_id().name().to_owned())) .collect(); - verify_modules(&modules)?; + verify_modules(&modules, VerifyFlag::ForPublish)?; Ok(modules) } @@ -147,17 +148,22 @@ pub fn build_and_verify_user_package(path: &Path, dev_mode: bool) -> SuiResult { ..Default::default() }; let modules = build_move_package(path, build_config, false)?; - verify_modules(&modules) + let flag = if dev_mode { + VerifyFlag::ForDev + } else { + VerifyFlag::ForPublish + }; + verify_modules(&modules, flag) } -fn verify_modules(modules: &[CompiledModule]) -> SuiResult { +fn verify_modules(modules: &[CompiledModule], flag: VerifyFlag) -> SuiResult { for m in modules { move_bytecode_verifier::verify_module(m).map_err(|err| { SuiError::ModuleVerificationFailure { error: err.to_string(), } })?; - sui_bytecode_verifier::verify_module(m)?; + sui_bytecode_verifier::verify_module(m, flag)?; } Ok(()) // TODO(https://github.com/MystenLabs/fastnft/issues/69): Run Move linker diff --git a/sui_programmability/verifier/src/lib.rs b/sui_programmability/verifier/src/lib.rs index 691af1a19825a..21eb97d4cb636 100644 --- a/sui_programmability/verifier/src/lib.rs +++ b/sui_programmability/verifier/src/lib.rs @@ -7,6 +7,7 @@ pub mod global_storage_access_verifier; pub mod id_immutable_verifier; pub mod id_leak_verifier; pub mod param_typecheck_verifier; +pub mod publish_verifier; pub mod struct_with_key_verifier; use sui_types::error::SuiError; diff --git a/sui_programmability/verifier/src/publish_verifier.rs b/sui_programmability/verifier/src/publish_verifier.rs new file mode 100644 index 0000000000000..d97990db623bd --- /dev/null +++ b/sui_programmability/verifier/src/publish_verifier.rs @@ -0,0 +1,52 @@ +// Copyright (c) 2022, Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//! This verifier only runs when we are trying to publish a module onchain. +//! Hence it contains publish-specific verification rules, such as no calls to Debug module. + +use crate::verification_failure; +use move_binary_format::{ + binary_views::BinaryIndexedView, + file_format::{Bytecode, CompiledModule, FunctionHandleIndex}, +}; +use sui_types::{error::SuiResult, SUI_FRAMEWORK_ADDRESS}; + +pub fn verify_module(module: &CompiledModule) -> SuiResult { + verify_no_debug_calls(module) +} + +/// Checks that whether any debug code is used when publishing a module onchain. +fn verify_no_debug_calls(module: &CompiledModule) -> SuiResult { + let view = BinaryIndexedView::Module(module); + for func_def in &module.function_defs { + if func_def.code.is_none() { + continue; + } + let code = &func_def.code.as_ref().unwrap().code; + for bytecode in code { + match bytecode { + Bytecode::Call(idx) => check_call(&view, idx)?, + Bytecode::CallGeneric(idx) => { + check_call(&view, &view.function_instantiation_at(*idx).handle)? + } + _ => {} + } + } + } + Ok(()) +} + +fn check_call(view: &BinaryIndexedView, func_idx: &FunctionHandleIndex) -> SuiResult { + let handle = view.function_handle_at(*func_idx); + let module_idx = handle.module; + let module_handle = view.module_handle_at(module_idx); + if view.address_identifier_at(module_handle.address) == &SUI_FRAMEWORK_ADDRESS + && view.identifier_at(module_handle.name).as_str() == "Debug" + { + Err(verification_failure( + format!("Calls to Debug module not allowed when publishing code onchain. Found in function '{:?}'", view.identifier_at(handle.name)) + )) + } else { + Ok(()) + } +} diff --git a/sui_programmability/verifier/src/verifier.rs b/sui_programmability/verifier/src/verifier.rs index 3f7e07309ecd5..644c4e6a28a6e 100644 --- a/sui_programmability/verifier/src/verifier.rs +++ b/sui_programmability/verifier/src/verifier.rs @@ -8,14 +8,24 @@ use sui_types::error::SuiResult; use crate::{ global_storage_access_verifier, id_immutable_verifier, id_leak_verifier, - param_typecheck_verifier, struct_with_key_verifier, + param_typecheck_verifier, publish_verifier, struct_with_key_verifier, }; +#[derive(Clone, Copy, PartialEq)] +pub enum VerifyFlag { + ForDev, + ForPublish, +} + /// Helper for a "canonical" verification of a module. -pub fn verify_module(module: &CompiledModule) -> SuiResult { +pub fn verify_module(module: &CompiledModule, flag: VerifyFlag) -> SuiResult { struct_with_key_verifier::verify_module(module)?; global_storage_access_verifier::verify_module(module)?; id_immutable_verifier::verify_module(module)?; id_leak_verifier::verify_module(module)?; - param_typecheck_verifier::verify_module(module) + param_typecheck_verifier::verify_module(module)?; + if flag == VerifyFlag::ForPublish { + publish_verifier::verify_module(module)?; + } + Ok(()) } diff --git a/sui_programmability/verifier/tests/common/module_builder.rs b/sui_programmability/verifier/tests/common/module_builder.rs index 7de79a2102621..776581236f794 100644 --- a/sui_programmability/verifier/tests/common/module_builder.rs +++ b/sui_programmability/verifier/tests/common/module_builder.rs @@ -21,6 +21,11 @@ pub struct FuncInfo { pub def: FunctionDefinitionIndex, } +pub struct GenericFuncInfo { + pub handle: FunctionInstantiationIndex, + pub def: FunctionDefinitionIndex, +} + impl ModuleBuilder { pub fn new(address: AccountAddress, name: &str) -> Self { Self { @@ -124,6 +129,30 @@ impl ModuleBuilder { ) } + pub fn add_generic_function( + &mut self, + module_idx: ModuleHandleIndex, + name: &str, + type_parameters: Vec, + parameters: Vec, + ret: Vec, + ) -> GenericFuncInfo { + let func_info = self.add_function(module_idx, name, parameters, ret); + let sig = self.add_signature(type_parameters); + let handle_idx = + FunctionInstantiationIndex(self.module.function_instantiations.len() as u16); + self.module + .function_instantiations + .push(FunctionInstantiation { + handle: func_info.handle, + type_parameters: sig, + }); + GenericFuncInfo { + handle: handle_idx, + def: func_info.def, + } + } + pub fn add_struct_verbose( &mut self, module_index: ModuleHandleIndex, diff --git a/sui_programmability/verifier/tests/publish_verification_test.rs b/sui_programmability/verifier/tests/publish_verification_test.rs new file mode 100644 index 0000000000000..afb6bed5c740d --- /dev/null +++ b/sui_programmability/verifier/tests/publish_verification_test.rs @@ -0,0 +1,36 @@ +// Copyright (c) 2021, Facebook, Inc. and its affiliates +// Copyright (c) 2022, Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +mod common; + +pub use common::module_builder::ModuleBuilder; +use move_binary_format::file_format::*; +use sui_types::SUI_FRAMEWORK_ADDRESS; +use sui_verifier::publish_verifier::verify_module; + +#[test] +fn function_with_debug_call() { + let (mut module, _) = ModuleBuilder::default(); + // Add mock Debug module. + let debug_module = module.add_module(SUI_FRAMEWORK_ADDRESS, "Debug"); + let print_func1 = module.add_function(debug_module, "print1", vec![], vec![]); + let print_func2 = module.add_generic_function(debug_module, "print2", vec![], vec![], vec![]); + let func = module.add_function(module.get_self_index(), "foo", vec![], vec![]); + assert!(verify_module(module.get_module()).is_ok()); + + // Bytecode that contains a call to Debug::print. + let code = vec![Bytecode::Call(print_func1.handle)]; + module.set_bytecode(func.def, code); + assert!(verify_module(module.get_module()) + .unwrap_err() + .to_string() + .contains("Calls to Debug module not allowed when publishing code onchain")); + + let code = vec![Bytecode::CallGeneric(print_func2.handle)]; + module.set_bytecode(func.def, code); + assert!(verify_module(module.get_module()) + .unwrap_err() + .to_string() + .contains("Calls to Debug module not allowed when publishing code onchain")); +}