diff --git a/substrate/frame/contracts/fixtures/invalid_import.wat b/substrate/frame/contracts/fixtures/invalid_import.wat new file mode 100644 index 0000000000..011f1a40e7 --- /dev/null +++ b/substrate/frame/contracts/fixtures/invalid_import.wat @@ -0,0 +1,6 @@ +;; A valid contract which does nothing at all but imports an invalid function +(module + (import "invalid" "invalid_88_99" (func (param i32 i32 i32))) + (func (export "deploy")) + (func (export "call")) +) diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index e4a54362c9..24d3a7f10e 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -632,6 +632,10 @@ pub mod pallet { /// or via RPC an `Ok` will be returned. In this case the caller needs to inspect the flags /// to determine whether a reversion has taken place. ContractReverted, + /// The contract's code was found to be invalid during validation or instrumentation. + /// A more detailed error can be found on the node console if debug messages are enabled + /// or in the debug buffer which is returned to RPC clients. + CodeRejected, } /// A mapping from an original code hash to the original code, untouched by instrumentation. @@ -781,7 +785,8 @@ where storage_deposit_limit: Option>, ) -> CodeUploadResult, BalanceOf> { let schedule = T::Schedule::get(); - let module = PrefabWasmModule::from_code(code, &schedule, origin)?; + let module = PrefabWasmModule::from_code(code, &schedule, origin) + .map_err(|_| >::CodeRejected)?; let deposit = module.open_deposit(); if let Some(storage_deposit_limit) = storage_deposit_limit { ensure!(storage_deposit_limit >= deposit, >::StorageDepositLimitExhausted); @@ -879,7 +884,7 @@ where code: Code>, data: Vec, salt: Vec, - debug_message: Option<&mut Vec>, + mut debug_message: Option<&mut Vec>, ) -> InternalInstantiateOutput { let mut storage_deposit = Default::default(); let mut gas_meter = GasMeter::new(gas_limit); @@ -891,8 +896,11 @@ where binary.len() as u32 <= schedule.limits.code_len, >::CodeTooLarge ); - let executable = - PrefabWasmModule::from_code(binary, &schedule, origin.clone())?; + let executable = PrefabWasmModule::from_code(binary, &schedule, origin.clone()) + .map_err(|msg| { + debug_message.as_mut().map(|buffer| buffer.extend(msg.as_bytes())); + >::CodeRejected + })?; ensure!( executable.code_len() <= schedule.limits.code_len, >::CodeTooLarge diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index be9904e71c..fd5c8cfd34 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -1386,7 +1386,7 @@ fn disabled_chain_extension_wont_deploy() { vec![], vec![], ), - "module uses chain extensions but chain extensions are disabled", + >::CodeRejected, ); }); } @@ -2903,3 +2903,32 @@ fn contract_reverted() { assert_eq!(result.data.0, buffer); }); } + +#[test] +fn code_rejected_error_works() { + let (wasm, _) = compile_module::("invalid_import").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + + assert_noop!( + Contracts::upload_code(Origin::signed(ALICE), wasm.clone(), None), + >::CodeRejected, + ); + + let result = Contracts::bare_instantiate( + ALICE, + 0, + GAS_LIMIT, + None, + Code::Upload(Bytes(wasm)), + vec![], + vec![], + true, + ); + assert_err!(result.result, >::CodeRejected); + assert_eq!( + std::str::from_utf8(&result.debug_message).unwrap(), + "module imports a non-existent function" + ); + }); +} diff --git a/substrate/frame/contracts/src/wasm/mod.rs b/substrate/frame/contracts/src/wasm/mod.rs index f63361a039..ee778982cd 100644 --- a/substrate/frame/contracts/src/wasm/mod.rs +++ b/substrate/frame/contracts/src/wasm/mod.rs @@ -122,8 +122,8 @@ where original_code: Vec, schedule: &Schedule, owner: AccountIdOf, - ) -> Result { - prepare::prepare_contract(original_code, schedule, owner).map_err(Into::into) + ) -> Result { + prepare::prepare_contract(original_code, schedule, owner) } /// Store the code without instantiating it. diff --git a/substrate/frame/contracts/src/wasm/prepare.rs b/substrate/frame/contracts/src/wasm/prepare.rs index 5e5f8e7e6f..a57e9aabf8 100644 --- a/substrate/frame/contracts/src/wasm/prepare.rs +++ b/substrate/frame/contracts/src/wasm/prepare.rs @@ -372,26 +372,34 @@ fn check_and_instrument( original_code: &[u8], schedule: &Schedule, ) -> Result<(Vec, (u32, u32)), &'static str> { - let contract_module = ContractModule::new(&original_code, schedule)?; - contract_module.scan_exports()?; - contract_module.ensure_no_internal_memory()?; - contract_module.ensure_table_size_limit(schedule.limits.table_size)?; - contract_module.ensure_global_variable_limit(schedule.limits.globals)?; - contract_module.ensure_no_floating_types()?; - contract_module.ensure_parameter_limit(schedule.limits.parameters)?; - contract_module.ensure_br_table_size_limit(schedule.limits.br_table_size)?; + let result = (|| { + let contract_module = ContractModule::new(&original_code, schedule)?; + contract_module.scan_exports()?; + contract_module.ensure_no_internal_memory()?; + contract_module.ensure_table_size_limit(schedule.limits.table_size)?; + contract_module.ensure_global_variable_limit(schedule.limits.globals)?; + contract_module.ensure_no_floating_types()?; + contract_module.ensure_parameter_limit(schedule.limits.parameters)?; + contract_module.ensure_br_table_size_limit(schedule.limits.br_table_size)?; - // We disallow importing `gas` function here since it is treated as implementation detail. - let disallowed_imports = [b"gas".as_ref()]; - let memory_limits = - get_memory_limits(contract_module.scan_imports::(&disallowed_imports)?, schedule)?; + // We disallow importing `gas` function here since it is treated as implementation detail. + let disallowed_imports = [b"gas".as_ref()]; + let memory_limits = + get_memory_limits(contract_module.scan_imports::(&disallowed_imports)?, schedule)?; - let code = contract_module - .inject_gas_metering()? - .inject_stack_height_metering()? - .into_wasm_code()?; + let code = contract_module + .inject_gas_metering()? + .inject_stack_height_metering()? + .into_wasm_code()?; - Ok((code, memory_limits)) + Ok((code, memory_limits)) + })(); + + if let Err(msg) = &result { + log::debug!(target: "runtime::contracts", "CodeRejected: {}", msg); + } + + result } fn do_preparation(