diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs index 67a8d20eb04c0..b0d7cc4c172f3 100644 --- a/aptos-move/aptos-release-builder/src/components/feature_flags.rs +++ b/aptos-move/aptos-release-builder/src/components/feature_flags.rs @@ -126,6 +126,7 @@ pub enum FeatureFlag { UseCompatibilityCheckerV2, EnableEnumTypes, EnableResourceAccessControl, + RejectUnstableBytecodeForScript, } fn generate_features_blob(writer: &CodeWriter, data: &[u64]) { @@ -332,6 +333,9 @@ impl From for AptosFeatureFlag { FeatureFlag::EnableResourceAccessControl => { AptosFeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL }, + FeatureFlag::RejectUnstableBytecodeForScript => { + AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT + }, } } } @@ -467,6 +471,9 @@ impl From for FeatureFlag { AptosFeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL => { FeatureFlag::EnableResourceAccessControl }, + AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT => { + FeatureFlag::RejectUnstableBytecodeForScript + }, } } } diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index b573feccc2e0f..00050681bff9c 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -90,6 +90,7 @@ use move_binary_format::{ compatibility::Compatibility, deserializer::DeserializerConfig, errors::{Location, PartialVMError, PartialVMResult, VMError, VMResult}, + file_format::CompiledScript, CompiledModule, }; use move_core_types::{ @@ -738,11 +739,30 @@ impl AptosVM { let func = session.load_script(script.code(), script.ty_args())?; - // TODO(Gerardo): consolidate the extended validation to verifier. - verifier::event_validation::verify_no_event_emission_in_script( + let compiled_script = match CompiledScript::deserialize_with_config( script.code(), self.deserializer_config(), - )?; + ) { + Ok(script) => script, + Err(err) => { + let msg = format!("[VM] deserializer for script returned error: {:?}", err); + let partial_err = PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) + .with_message(msg) + .finish(Location::Script); + return Err(partial_err.into_vm_status()); + }, + }; + + // Check that unstable bytecode cannot be executed on mainnet + if self + .features() + .is_enabled(FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT) + { + self.reject_unstable_bytecode_for_script(&compiled_script)?; + } + + // TODO(Gerardo): consolidate the extended validation to verifier. + verifier::event_validation::verify_no_event_emission_in_compiled_script(&compiled_script)?; let args = verifier::transaction_arg_validation::validate_combine_signer_and_txn_args( session, @@ -1622,6 +1642,22 @@ impl AptosVM { Ok(()) } + /// Check whether the script can be run on mainnet based on the unstable tag in the metadata + pub fn reject_unstable_bytecode_for_script(&self, module: &CompiledScript) -> VMResult<()> { + if self.chain_id().is_mainnet() { + if let Some(metadata) = + aptos_framework::get_compilation_metadata_from_compiled_script(module) + { + if metadata.unstable { + return Err(PartialVMError::new(StatusCode::UNSTABLE_BYTECODE_REJECTED) + .with_message("script marked unstable cannot be run on mainnet".to_string()) + .finish(Location::Script)); + } + } + } + Ok(()) + } + fn metadata_validation_error(msg: &str) -> VMError { PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED) .with_message(format!("metadata and code bundle mismatch: {}", msg)) diff --git a/aptos-move/aptos-vm/src/verifier/event_validation.rs b/aptos-move/aptos-vm/src/verifier/event_validation.rs index 6b1e2fb1e10e8..761a94bb2652d 100644 --- a/aptos-move/aptos-vm/src/verifier/event_validation.rs +++ b/aptos-move/aptos-vm/src/verifier/event_validation.rs @@ -5,7 +5,6 @@ use crate::move_vm_ext::SessionExt; use aptos_framework::RuntimeModuleMetadataV1; use move_binary_format::{ access::{ModuleAccess, ScriptAccess}, - deserializer::DeserializerConfig, errors::{Location, PartialVMError, VMError, VMResult}, file_format::{ Bytecode, CompiledScript, @@ -151,19 +150,7 @@ pub(crate) fn extract_event_metadata( Ok(event_structs) } -pub(crate) fn verify_no_event_emission_in_script( - script_code: &[u8], - config: &DeserializerConfig, -) -> VMResult<()> { - let script = match CompiledScript::deserialize_with_config(script_code, config) { - Ok(script) => script, - Err(err) => { - let msg = format!("[VM] deserializer for script returned error: {:?}", err); - return Err(PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR) - .with_message(msg) - .finish(Location::Script)); - }, - }; +pub(crate) fn verify_no_event_emission_in_compiled_script(script: &CompiledScript) -> VMResult<()> { for bc in &script.code().code { if let Bytecode::CallGeneric(index) = bc { let func_instantiation = &script.function_instantiation_at(*index); diff --git a/aptos-move/e2e-move-tests/src/tests/metadata.rs b/aptos-move/e2e-move-tests/src/tests/metadata.rs index 7c499fe762bcd..60037dbb08518 100644 --- a/aptos-move/e2e-move-tests/src/tests/metadata.rs +++ b/aptos-move/e2e-move-tests/src/tests/metadata.rs @@ -14,7 +14,7 @@ use aptos_package_builder::PackageBuilder; use aptos_types::{ chain_id::ChainId, on_chain_config::{FeatureFlag, OnChainConfig}, - transaction::TransactionStatus, + transaction::{Script, TransactionPayload, TransactionStatus}, }; use move_binary_format::CompiledModule; use move_core_types::{ @@ -269,6 +269,118 @@ fn test_compilation_metadata_internal( } } +fn test_compilation_metadata_script_internal( + mainnet_flag: bool, + v2_flag: bool, + feature_enabled: bool, +) -> TransactionStatus { + let mut h = MoveHarness::new(); + if feature_enabled { + h.enable_features( + vec![FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT], + vec![], + ); + } else { + h.enable_features(vec![], vec![ + FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT, + ]); + } + let account = h.new_account_at(AccountAddress::from_hex_literal("0xf00d").unwrap()); + let mut builder = PackageBuilder::new("Package"); + builder.add_source( + "m.move", + r#" + script { + fun main() { } + } + "#, + ); + let path = builder.write_to_temp().unwrap(); + + let compiler_version = if v2_flag { + CompilerVersion::V2_0 + } else { + CompilerVersion::V1 + }; + let package = build_package_with_compiler_version( + path.path().to_path_buf(), + BuildOptions::default(), + compiler_version, + ) + .expect("building package must succeed"); + + let code = package.extract_script_code().into_iter().next().unwrap(); + + let script = TransactionPayload::Script(Script::new(code, vec![], vec![])); + + if mainnet_flag { + h.set_resource( + CORE_CODE_ADDRESS, + ChainId::struct_tag(), + &ChainId::mainnet().id(), + ); + h.run_transaction_payload_mainnet(&account, script) + } else { + h.run_transaction_payload(&account, script) + } +} + +#[test] +fn test_compilation_metadata_for_script() { + let mut enable_check = true; + // run compiler v2 code to mainnet + assert_vm_status!( + test_compilation_metadata_script_internal(true, true, enable_check), + StatusCode::UNSTABLE_BYTECODE_REJECTED + ); + // run compiler v1 code to mainnet + assert_success!(test_compilation_metadata_script_internal( + true, + false, + enable_check + )); + // run compiler v2 code to test + assert_success!(test_compilation_metadata_script_internal( + false, + true, + enable_check + )); + // run compiler v1 code to test + assert_success!(test_compilation_metadata_script_internal( + false, + false, + enable_check + )); + + enable_check = false; + // run compiler v2 code to mainnet + // success because the feature flag is turned off + assert_success!(test_compilation_metadata_script_internal( + true, + true, + enable_check + ),); + // run compiler v1 code to mainnet + assert_success!(test_compilation_metadata_script_internal( + true, + false, + enable_check + )); + // run compiler v2 code to test + // success because the feature flag is turned off + assert_success!(test_compilation_metadata_script_internal( + false, + true, + enable_check + ),); + // run compiler v1 code to test + assert_success!(test_compilation_metadata_script_internal( + false, + false, + enable_check + )); +} + #[test] fn test_compilation_metadata() { let mut enable_check = true; diff --git a/aptos-move/framework/src/module_metadata.rs b/aptos-move/framework/src/module_metadata.rs index 217e50e354a68..e0dc1d36b4fa6 100644 --- a/aptos-move/framework/src/module_metadata.rs +++ b/aptos-move/framework/src/module_metadata.rs @@ -312,6 +312,17 @@ pub fn get_compilation_metadata_from_compiled_module( } } +/// Extract compilation metadata from a compiled script +pub fn get_compilation_metadata_from_compiled_script( + module: &CompiledScript, +) -> Option { + if let Some(data) = find_metadata_in_script(module, COMPILATION_METADATA_KEY) { + bcs::from_bytes::(&data.value).ok() + } else { + None + } +} + // This is mostly a copy paste of the existing function // get_metadata_from_compiled_module. In the API types there is a unifying trait for // modules and scripts called Bytecode that could help eliminate this duplication, diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs index 653f173e57c15..79d2045b551ce 100644 --- a/types/src/on_chain_config/aptos_features.rs +++ b/types/src/on_chain_config/aptos_features.rs @@ -91,6 +91,7 @@ pub enum FeatureFlag { USE_COMPATIBILITY_CHECKER_V2 = 73, ENABLE_ENUM_TYPES = 74, ENABLE_RESOURCE_ACCESS_CONTROL = 75, + REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT = 76, } impl FeatureFlag { @@ -165,6 +166,7 @@ impl FeatureFlag { FeatureFlag::USE_COMPATIBILITY_CHECKER_V2, FeatureFlag::ENABLE_ENUM_TYPES, FeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL, + FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT, ] } }