From 7f3028e88fcce850c9bccbaadfdec82f1e922160 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 4 Feb 2021 07:18:19 +0100 Subject: [PATCH 01/18] Implement Wasm validation for known issues/markers --- src/cmd/build.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index adf88e90..7c694a3f 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -36,6 +36,14 @@ use structopt::StructOpt; /// This is the maximum number of pages available for a contract to allocate. const MAX_MEMORY_PAGES: u32 = 16; +/// Marker inserted by the ink! codegen for a cross-calling trait error which +/// can't be checked at compile time. +const INK_ENFORCE_ERR_MESSAGE: &str = "ink_enforce_error_for_message_"; + +/// Marker inserted by the ink! codegen for a cross-calling trait constructor +/// error which can't be checked at compile time. +const INK_ENFORCE_ERR_CONSTRUCTOR: &str = "ink_enforce_error_for_constructor_"; + /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. /// /// It does so by invoking `cargo build` and then post processing the final binary. @@ -249,10 +257,74 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { ensure_maximum_memory_pages(&mut module, MAX_MEMORY_PAGES)?; strip_custom_sections(&mut module); + validate_import_section(&module)?; + parity_wasm::serialize_to_file(&crate_metadata.dest_wasm, module)?; Ok(()) } +/// Validates the import section in the Wasm. +/// +/// The checks currently fall into two categories: +/// - Known bugs for which we want to recommend a solution. +/// - Markers inserted by the ink! codegen for errors which can't be checked at compile time. +fn validate_import_section(module: &Module) -> Result<()> { + let imports = module + .import_section() + .expect("import section must exist") + .entries() + .iter(); + let original_imports_len = imports.len(); + let mut errs = Vec::new(); + + let filtered_imports = imports.filter(|section| { + let field = section.field(); + if field.contains("panic") { + errs.push(String::from( + "An unexpected panic function import was found in the contract Wasm.\n\ + This typically goes back to a known bug in the Rust compiler:\n\ + https://github.com/rust-lang/rust/issues/78744\n\n\ + As a workaround try to insert `overflow-checks = false` into your `Cargo.toml`.\n\ + This will disable safe math operations, but unfortunately we are currently not \n\ + aware of a better workaround until the bug in the compiler is fixed.", + )); + } else if field.contains(INK_ENFORCE_ERR_MESSAGE) { + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! message with the selector `{}` contains an invalid trait call.\n\n\ + Please check if the mutable parameter of the function to call is consistent\n\ + with the scope in which it is called.", + field + .strip_prefix(INK_ENFORCE_ERR_MESSAGE) + .expect("must exist") + )); + } else if field.contains(INK_ENFORCE_ERR_CONSTRUCTOR) { + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! constructor with the selector `{}` contains an invalid trait call.\n\ + Constructor never need to be forwarded, please check if this is the case.", + field + .strip_prefix(INK_ENFORCE_ERR_CONSTRUCTOR) + .expect("must exist") + )); + } + + // the only allowed imports + field.contains("seal") | field.contains("memory") + }); + + if original_imports_len != filtered_imports.count() { + anyhow::bail!(format!( + "Validation of the Wasm failed.\n\n\n{}", + errs.into_iter() + .map(|err| format!("{} {}", "ERROR:".to_string().bold(), err)) + .collect::>() + .join("\n\n\n") + )); + } + Ok(()) +} + /// Attempts to perform optional wasm optimization using `binaryen`. /// /// The intention is to reduce the size of bloated wasm binaries as a result of missing -- GitLab From a12e35e62354aa67d3f2e7580c1f93b125b825d5 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Fri, 5 Feb 2021 15:26:53 +0100 Subject: [PATCH 02/18] Add test for invalid panic import --- src/cmd/build.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 7c694a3f..5605cce1 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -448,7 +448,7 @@ pub(crate) fn execute_with_crate_metadata( #[cfg(feature = "test-ci-only")] #[cfg(test)] -mod tests { +mod tests_ci_only { use crate::{cmd, util::tests::with_tmp_dir, BuildArtifacts, ManifestPath, UnstableFlags}; #[test] @@ -503,3 +503,31 @@ mod tests { }) } } + +#[cfg(test)] +mod tests { + use crate::cmd::build::validate_import_section; + + #[test] + fn validate_must_catch_panic_import() { + // given + let contract = r#" + (module + (type (;0;) (func (param i32 i32 i32))) + (import "env" "_ZN4core9panicking5panic17h00e3acdd8048cb7cE" (func (;5;) (type 0))) + (func (;5;) (type 0)) + )"#; + let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); + let module = parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work"); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + assert!(res + .unwrap_err() + .to_string() + .contains("An unexpected panic function import was found in the contract Wasm.")); + } +} -- GitLab From eb10d0da9e5585aa1ebc49d3528301c0508e1e52 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Fri, 5 Feb 2021 15:28:35 +0100 Subject: [PATCH 03/18] Add prefix to error markers --- src/cmd/build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 5605cce1..dd858f1e 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -38,11 +38,11 @@ const MAX_MEMORY_PAGES: u32 = 16; /// Marker inserted by the ink! codegen for a cross-calling trait error which /// can't be checked at compile time. -const INK_ENFORCE_ERR_MESSAGE: &str = "ink_enforce_error_for_message_"; +const INK_ENFORCE_ERR_MESSAGE: &str = "__ink_enforce_error_for_message_"; /// Marker inserted by the ink! codegen for a cross-calling trait constructor /// error which can't be checked at compile time. -const INK_ENFORCE_ERR_CONSTRUCTOR: &str = "ink_enforce_error_for_constructor_"; +const INK_ENFORCE_ERR_CONSTRUCTOR: &str = "__ink_enforce_error_for_constructor_"; /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. /// -- GitLab From 3c4224c43b189adf220568693a56ce9fecb8c8c8 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Fri, 5 Feb 2021 16:17:06 +0100 Subject: [PATCH 04/18] Add test for ink! codegen error marker --- src/cmd/build.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index dd858f1e..f45d2e18 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -530,4 +530,30 @@ mod tests { .to_string() .contains("An unexpected panic function import was found in the contract Wasm.")); } + + #[test] + fn validate_must_catch_ink_enforce_error_markers() { + // given + let contract = r#" + (module + (type (;0;) (func)) + (import "env" "__ink_enforce_error_for_message_0xAD931DAA" (func $__ink_enforce_error_for_message_0xAD931DAA (type 0))) + (import "env" "__ink_enforce_error_for_constructor_0xAD931DCC" (func $__ink_enforce_error_for_message_0xAD931DCC (type 0))) + )"#; + let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); + let module = parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work"); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + let err = res.unwrap_err().to_string(); + assert!(err.contains( + "The ink! message with the selector `0xAD931DAA` contains an invalid trait call." + )); + assert!(err.contains( + "The ink! constructor with the selector `0xAD931DCC` contains an invalid trait call." + )); + } } -- GitLab From fa6273f1113bdfd7e4dbb988858c909fda84ef58 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 07:48:31 +0100 Subject: [PATCH 05/18] Implement improved linker error protocol --- Cargo.lock | 16 ++++++- Cargo.toml | 3 +- src/cmd/build.rs | 106 ++++++++++++++++++++++++++++++++++++----------- 3 files changed, 98 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab1452e8..a169fee7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -578,6 +578,7 @@ dependencies = [ "futures 0.3.12", "heck", "hex", + "impl-serde", "log", "parity-scale-codec 2.0.0", "parity-wasm 0.42.1", @@ -2434,7 +2435,7 @@ dependencies = [ "arrayvec 0.5.2", "bitvec 0.17.4", "byte-slice-cast 0.3.5", - "parity-scale-codec-derive", + "parity-scale-codec-derive 1.2.2", "serde", ] @@ -2447,6 +2448,7 @@ dependencies = [ "arrayvec 0.5.2", "bitvec 0.20.1", "byte-slice-cast 1.0.0", + "parity-scale-codec-derive 2.0.0", "serde", ] @@ -2462,6 +2464,18 @@ dependencies = [ "syn", ] +[[package]] +name = "parity-scale-codec-derive" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9029e65297c7fd6d7013f0579e193ec2b34ae78eabca854c9417504ad8a2d214" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "parity-util-mem" version = "0.7.0" diff --git a/Cargo.toml b/Cargo.toml index fed702dd..763a6c8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ zip = { version = "0.5.8", default-features = false } pwasm-utils = "0.17.0" parity-wasm = "0.42.1" cargo_metadata = "0.12.3" -codec = { package = "parity-scale-codec", version = "2.0.0" } +codec = { package = "parity-scale-codec", version = "2.0.0", features = ["derive"] } which = "4.0.2" colored = "2.0.0" toml = "0.5.8" @@ -41,6 +41,7 @@ serde_json = "1.0.61" tempfile = "3.2.0" url = { version = "2.2.0", features = ["serde"] } binaryen = "0.12.0" +impl-serde = "0.3.1" # dependencies for optional extrinsics feature async-std = { version = "1.9.0", optional = true } diff --git a/src/cmd/build.rs b/src/cmd/build.rs index f45d2e18..292eba11 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with cargo-contract. If not, see . +use impl_serde::serialize as serde_hex; use std::{ convert::TryFrom, fs::{metadata, File}, @@ -36,13 +37,49 @@ use structopt::StructOpt; /// This is the maximum number of pages available for a contract to allocate. const MAX_MEMORY_PAGES: u32 = 16; -/// Marker inserted by the ink! codegen for a cross-calling trait error which -/// can't be checked at compile time. -const INK_ENFORCE_ERR_MESSAGE: &str = "__ink_enforce_error_for_message_"; +/// Marker inserted by the ink! codegen for an error which can't +/// be checked at compile time. +//const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; +const INK_ENFORCE_ERR: &str = "ink_enforce_error_"; -/// Marker inserted by the ink! codegen for a cross-calling trait constructor -/// error which can't be checked at compile time. -const INK_ENFORCE_ERR_CONSTRUCTOR: &str = "__ink_enforce_error_for_constructor_"; +/// Errors which may occur when forwarding a call is not allowed. +/// +/// We insert markers for these errors in the generated contract code. +/// This is necessary since we can't check these errors at compile time +/// of the contract. +/// `cargo-contract` checks the contract code for these error markers +/// when building a contract and fails if it finds markers. +#[derive(codec::Encode, codec::Decode)] +pub enum EnforcedErrors { + /// The below error represents calling a `&mut self` message in a context that + /// only allows for `&self` messages. This may happen under certain circumstances + /// when ink! trait implementations are involved with long-hand calling notation. + #[codec(index = 1)] + CannotCallTraitMessage { + /// The trait that defines the called message. + trait_ident: String, + /// The name of the called message. + message_ident: String, + /// The selector of the called message. + message_selector: [u8; 4], + /// Is `true` if the `self` receiver of the ink! message is `&mut self`. + message_mut: bool, + }, + /// The below error represents calling a constructor in a context that does + /// not allow calling it. This may happen when the constructor defined in a + /// trait is cross-called in another contract. + /// This is not allowed since the contract to which a call is forwarded must + /// already exist at the point when the call to it is made. + #[codec(index = 2)] + CannotCallTraitConstructor { + /// The trait that defines the called constructor. + trait_ident: String, + /// The name of the called constructor. + constructor_ident: String, + /// The selector of the called constructor. + constructor_selector: [u8; 4], + }, +} /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. /// @@ -288,25 +325,44 @@ fn validate_import_section(module: &Module) -> Result<()> { This will disable safe math operations, but unfortunately we are currently not \n\ aware of a better workaround until the bug in the compiler is fixed.", )); - } else if field.contains(INK_ENFORCE_ERR_MESSAGE) { - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! message with the selector `{}` contains an invalid trait call.\n\n\ - Please check if the mutable parameter of the function to call is consistent\n\ - with the scope in which it is called.", - field - .strip_prefix(INK_ENFORCE_ERR_MESSAGE) - .expect("must exist") - )); - } else if field.contains(INK_ENFORCE_ERR_CONSTRUCTOR) { - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! constructor with the selector `{}` contains an invalid trait call.\n\ - Constructor never need to be forwarded, please check if this is the case.", - field - .strip_prefix(INK_ENFORCE_ERR_CONSTRUCTOR) - .expect("must exist") - )); + } else if field.contains(INK_ENFORCE_ERR) { + let encoded = field + .strip_prefix(INK_ENFORCE_ERR) + .expect("must exist"); + let hex = serde_hex::from_hex(&encoded) + .expect("decoding hex failed"); + let decoded = ::decode(&mut &hex[..]) + .expect("decoding object failed"); + match decoded { + EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { + let receiver = match message_mut { + true => "&mut self", + false => "&self", + }; + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ + Please check if the receiver of the function to call is consistent\n\ + with the scope in which it is called. The receiver is `{}`.", + trait_ident, + message_ident, + serde_hex::to_hex(&codec::Encode::encode(&message_selector), false) + receiver + )); + }, + EnforcedErrors::CannotCallTraitConstructor { + trait_ident, constructor_ident, constructor_selector + } => { + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ + Constructor never need to be forwarded, please check if this is the case.", + trait_ident, + constructor_ident, + serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) + )); + } + } } // the only allowed imports -- GitLab From 6c7a1e450da35b4134dbaa3e444e6e9ab8bbaa5f Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:14:20 +0100 Subject: [PATCH 06/18] Move validation into its own module --- src/cmd/build.rs | 129 +--------------------------- src/main.rs | 1 + src/validation/mod.rs | 17 ++++ src/validation/validate_wasm.rs | 145 ++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 127 deletions(-) create mode 100644 src/validation/mod.rs create mode 100644 src/validation/validate_wasm.rs diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 292eba11..a645efc9 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -14,7 +14,6 @@ // You should have received a copy of the GNU General Public License // along with cargo-contract. If not, see . -use impl_serde::serialize as serde_hex; use std::{ convert::TryFrom, fs::{metadata, File}, @@ -25,6 +24,7 @@ use std::{ use crate::{ crate_metadata::CrateMetadata, maybe_println, util, + validation::validate_wasm, workspace::{ManifestPath, Profile, Workspace}, BuildArtifacts, BuildResult, UnstableFlags, UnstableOptions, VerbosityFlags, }; @@ -37,50 +37,6 @@ use structopt::StructOpt; /// This is the maximum number of pages available for a contract to allocate. const MAX_MEMORY_PAGES: u32 = 16; -/// Marker inserted by the ink! codegen for an error which can't -/// be checked at compile time. -//const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; -const INK_ENFORCE_ERR: &str = "ink_enforce_error_"; - -/// Errors which may occur when forwarding a call is not allowed. -/// -/// We insert markers for these errors in the generated contract code. -/// This is necessary since we can't check these errors at compile time -/// of the contract. -/// `cargo-contract` checks the contract code for these error markers -/// when building a contract and fails if it finds markers. -#[derive(codec::Encode, codec::Decode)] -pub enum EnforcedErrors { - /// The below error represents calling a `&mut self` message in a context that - /// only allows for `&self` messages. This may happen under certain circumstances - /// when ink! trait implementations are involved with long-hand calling notation. - #[codec(index = 1)] - CannotCallTraitMessage { - /// The trait that defines the called message. - trait_ident: String, - /// The name of the called message. - message_ident: String, - /// The selector of the called message. - message_selector: [u8; 4], - /// Is `true` if the `self` receiver of the ink! message is `&mut self`. - message_mut: bool, - }, - /// The below error represents calling a constructor in a context that does - /// not allow calling it. This may happen when the constructor defined in a - /// trait is cross-called in another contract. - /// This is not allowed since the contract to which a call is forwarded must - /// already exist at the point when the call to it is made. - #[codec(index = 2)] - CannotCallTraitConstructor { - /// The trait that defines the called constructor. - trait_ident: String, - /// The name of the called constructor. - constructor_ident: String, - /// The selector of the called constructor. - constructor_selector: [u8; 4], - }, -} - /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. /// /// It does so by invoking `cargo build` and then post processing the final binary. @@ -294,93 +250,12 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { ensure_maximum_memory_pages(&mut module, MAX_MEMORY_PAGES)?; strip_custom_sections(&mut module); - validate_import_section(&module)?; + validate_wasm::validate_import_section(&module)?; parity_wasm::serialize_to_file(&crate_metadata.dest_wasm, module)?; Ok(()) } -/// Validates the import section in the Wasm. -/// -/// The checks currently fall into two categories: -/// - Known bugs for which we want to recommend a solution. -/// - Markers inserted by the ink! codegen for errors which can't be checked at compile time. -fn validate_import_section(module: &Module) -> Result<()> { - let imports = module - .import_section() - .expect("import section must exist") - .entries() - .iter(); - let original_imports_len = imports.len(); - let mut errs = Vec::new(); - - let filtered_imports = imports.filter(|section| { - let field = section.field(); - if field.contains("panic") { - errs.push(String::from( - "An unexpected panic function import was found in the contract Wasm.\n\ - This typically goes back to a known bug in the Rust compiler:\n\ - https://github.com/rust-lang/rust/issues/78744\n\n\ - As a workaround try to insert `overflow-checks = false` into your `Cargo.toml`.\n\ - This will disable safe math operations, but unfortunately we are currently not \n\ - aware of a better workaround until the bug in the compiler is fixed.", - )); - } else if field.contains(INK_ENFORCE_ERR) { - let encoded = field - .strip_prefix(INK_ENFORCE_ERR) - .expect("must exist"); - let hex = serde_hex::from_hex(&encoded) - .expect("decoding hex failed"); - let decoded = ::decode(&mut &hex[..]) - .expect("decoding object failed"); - match decoded { - EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { - let receiver = match message_mut { - true => "&mut self", - false => "&self", - }; - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ - Please check if the receiver of the function to call is consistent\n\ - with the scope in which it is called. The receiver is `{}`.", - trait_ident, - message_ident, - serde_hex::to_hex(&codec::Encode::encode(&message_selector), false) - receiver - )); - }, - EnforcedErrors::CannotCallTraitConstructor { - trait_ident, constructor_ident, constructor_selector - } => { - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ - Constructor never need to be forwarded, please check if this is the case.", - trait_ident, - constructor_ident, - serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) - )); - } - } - } - - // the only allowed imports - field.contains("seal") | field.contains("memory") - }); - - if original_imports_len != filtered_imports.count() { - anyhow::bail!(format!( - "Validation of the Wasm failed.\n\n\n{}", - errs.into_iter() - .map(|err| format!("{} {}", "ERROR:".to_string().bold(), err)) - .collect::>() - .join("\n\n\n") - )); - } - Ok(()) -} - /// Attempts to perform optional wasm optimization using `binaryen`. /// /// The intention is to reduce the size of bloated wasm binaries as a result of missing diff --git a/src/main.rs b/src/main.rs index cd3295db..c178d84c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,6 +17,7 @@ mod cmd; mod crate_metadata; mod util; +mod validation; mod workspace; use self::workspace::ManifestPath; diff --git a/src/validation/mod.rs b/src/validation/mod.rs new file mode 100644 index 00000000..7846f72d --- /dev/null +++ b/src/validation/mod.rs @@ -0,0 +1,17 @@ +// Copyright 2018-2021 Parity Technologies (UK) Ltd. +// This file is part of cargo-contract. +// +// cargo-contract is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// cargo-contract is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with cargo-contract. If not, see . + +pub mod validate_wasm; diff --git a/src/validation/validate_wasm.rs b/src/validation/validate_wasm.rs new file mode 100644 index 00000000..ada2414f --- /dev/null +++ b/src/validation/validate_wasm.rs @@ -0,0 +1,145 @@ +// Copyright 2018-2021 Parity Technologies (UK) Ltd. +// This file is part of cargo-contract. +// +// cargo-contract is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// cargo-contract is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with cargo-contract. If not, see . + +use anyhow::Result; +use colored::Colorize; +use impl_serde::serialize as serde_hex; +use parity_wasm::elements::Module; + +/// Marker inserted by the ink! codegen for an error which can't +/// be checked at compile time. +//const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; +const INK_ENFORCE_ERR: &str = "ink_enforce_error_"; + +/// Errors which may occur when forwarding a call is not allowed. +/// +/// We insert markers for these errors in the generated contract code. +/// This is necessary since we can't check these errors at compile time +/// of the contract. +/// `cargo-contract` checks the contract code for these error markers +/// when building a contract and fails if it finds markers. +#[derive(codec::Encode, codec::Decode)] +pub enum EnforcedErrors { + /// The below error represents calling a `&mut self` message in a context that + /// only allows for `&self` messages. This may happen under certain circumstances + /// when ink! trait implementations are involved with long-hand calling notation. + #[codec(index = 1)] + CannotCallTraitMessage { + /// The trait that defines the called message. + trait_ident: String, + /// The name of the called message. + message_ident: String, + /// The selector of the called message. + message_selector: [u8; 4], + /// Is `true` if the `self` receiver of the ink! message is `&mut self`. + message_mut: bool, + }, + /// The below error represents calling a constructor in a context that does + /// not allow calling it. This may happen when the constructor defined in a + /// trait is cross-called in another contract. + /// This is not allowed since the contract to which a call is forwarded must + /// already exist at the point when the call to it is made. + #[codec(index = 2)] + CannotCallTraitConstructor { + /// The trait that defines the called constructor. + trait_ident: String, + /// The name of the called constructor. + constructor_ident: String, + /// The selector of the called constructor. + constructor_selector: [u8; 4], + }, +} + +/// Validates the import section in the Wasm. +/// +/// The checks currently fall into two categories: +/// - Known bugs for which we want to recommend a solution. +/// - Markers inserted by the ink! codegen for errors which can't be checked at compile time. +pub fn validate_import_section(module: &Module) -> Result<()> { + let imports = module + .import_section() + .expect("import section must exist") + .entries() + .iter(); + let original_imports_len = imports.len(); + let mut errs = Vec::new(); + + let filtered_imports = imports.filter(|section| { + let field = section.field(); + if field.contains("panic") { + errs.push(String::from( + "An unexpected panic function import was found in the contract Wasm.\n\ + This typically goes back to a known bug in the Rust compiler:\n\ + https://github.com/rust-lang/rust/issues/78744\n\n\ + As a workaround try to insert `overflow-checks = false` into your `Cargo.toml`.\n\ + This will disable safe math operations, but unfortunately we are currently not \n\ + aware of a better workaround until the bug in the compiler is fixed.", + )); + } else if field.contains(INK_ENFORCE_ERR) { + let encoded = field + .strip_prefix(INK_ENFORCE_ERR) + .expect("must exist"); + let hex = serde_hex::from_hex(&encoded) + .expect("decoding hex failed"); + let decoded = ::decode(&mut &hex[..]) + .expect("decoding object failed"); + match decoded { + EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { + let receiver = match message_mut { + true => "&mut self", + false => "&self", + }; + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ + Please check if the receiver of the function to call is consistent\n\ + with the scope in which it is called. The receiver is `{}`.", + trait_ident, + message_ident, + serde_hex::to_hex(&codec::Encode::encode(&message_selector), false), + receiver + )); + }, + EnforcedErrors::CannotCallTraitConstructor { + trait_ident, constructor_ident, constructor_selector + } => { + errs.push(format!( + "An error was found while compiling the contract:\n\ + The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ + Constructor never need to be forwarded, please check if this is the case.", + trait_ident, + constructor_ident, + serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) + )); + } + } + } + + // the only allowed imports + field.contains("seal") | field.contains("memory") + }); + + if original_imports_len != filtered_imports.count() { + anyhow::bail!(format!( + "Validation of the Wasm failed.\n\n\n{}", + errs.into_iter() + .map(|err| format!("{} {}", "ERROR:".to_string().bold(), err)) + .collect::>() + .join("\n\n\n") + )); + } + Ok(()) +} -- GitLab From 66b2953a67043a9b1856e3ce58c38f170af06e2a Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:30:45 +0100 Subject: [PATCH 07/18] Migrate tests to new linker error protocol --- src/cmd/build.rs | 54 ---------------------- src/validation/validate_wasm.rs | 79 ++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 56 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index a645efc9..e2a3875a 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -434,57 +434,3 @@ mod tests_ci_only { }) } } - -#[cfg(test)] -mod tests { - use crate::cmd::build::validate_import_section; - - #[test] - fn validate_must_catch_panic_import() { - // given - let contract = r#" - (module - (type (;0;) (func (param i32 i32 i32))) - (import "env" "_ZN4core9panicking5panic17h00e3acdd8048cb7cE" (func (;5;) (type 0))) - (func (;5;) (type 0)) - )"#; - let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); - let module = parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work"); - - // when - let res = validate_import_section(&module); - - // then - assert!(res.is_err()); - assert!(res - .unwrap_err() - .to_string() - .contains("An unexpected panic function import was found in the contract Wasm.")); - } - - #[test] - fn validate_must_catch_ink_enforce_error_markers() { - // given - let contract = r#" - (module - (type (;0;) (func)) - (import "env" "__ink_enforce_error_for_message_0xAD931DAA" (func $__ink_enforce_error_for_message_0xAD931DAA (type 0))) - (import "env" "__ink_enforce_error_for_constructor_0xAD931DCC" (func $__ink_enforce_error_for_message_0xAD931DCC (type 0))) - )"#; - let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); - let module = parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work"); - - // when - let res = validate_import_section(&module); - - // then - assert!(res.is_err()); - let err = res.unwrap_err().to_string(); - assert!(err.contains( - "The ink! message with the selector `0xAD931DAA` contains an invalid trait call." - )); - assert!(err.contains( - "The ink! constructor with the selector `0xAD931DCC` contains an invalid trait call." - )); - } -} diff --git a/src/validation/validate_wasm.rs b/src/validation/validate_wasm.rs index ada2414f..8c2f88ac 100644 --- a/src/validation/validate_wasm.rs +++ b/src/validation/validate_wasm.rs @@ -21,8 +21,7 @@ use parity_wasm::elements::Module; /// Marker inserted by the ink! codegen for an error which can't /// be checked at compile time. -//const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; -const INK_ENFORCE_ERR: &str = "ink_enforce_error_"; +const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; /// Errors which may occur when forwarding a call is not allowed. /// @@ -143,3 +142,79 @@ pub fn validate_import_section(module: &Module) -> Result<()> { } Ok(()) } + +#[cfg(test)] +mod tests { + use super::validate_import_section; + use parity_wasm::elements::Module; + + fn create_module(contract: &str) -> Module { + let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); + parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work") + } + + #[test] + fn validate_must_catch_panic_import() { + // given + let contract = r#" + (module + (type (;0;) (func (param i32 i32 i32))) + (import "env" "_ZN4core9panicking5panic17h00e3acdd8048cb7cE" (func (;5;) (type 0))) + (func (;5;) (type 0)) + )"#; + let module = create_module(contract); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + assert!(res + .unwrap_err() + .to_string() + .contains("An unexpected panic function import was found in the contract Wasm.")); + } + + #[test] + fn must_catch_ink_enforce_error_marker_message() { + // given + let contract = r#" + (module + (type (;0;) (func)) + (import "env" "__ink_enforce_error_0x0110466c697010666c6970aa97cade01" (func $__ink_enforce_error_0x0110466c697010666c6970aa97cade01 (type 0))) + )"#; + let wasm = wabt::wat2wasm(contract).expect("invalid wabt"); + let module = parity_wasm::deserialize_buffer(&wasm).expect("deserializing must work"); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + let err = res.unwrap_err().to_string(); + assert!(err.contains( + "The ink! message `Flip::flip` with the selector `0xaa97cade` contains an invalid trait call." + )); + assert!(err.contains("The receiver is `&mut self`.",)); + } + + #[test] + fn must_catch_ink_enforce_error_marker_constructor() { + // given + let contract = r#" + (module + (type (;0;) (func)) + (import "env" "__ink_enforce_error_0x0210466c69700c6e657740d75d74" (func $__ink_enforce_error_0x0210466c69700c6e657740d75d74 (type 0))) + )"#; + let module = create_module(contract); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + assert!(res.unwrap_err().to_string().contains( + "The ink! constructor `Flip::new` with the selector `0x40d75d74` contains an invalid trait call." + )); + } +} -- GitLab From 6fc65a8d767d416e273338ad5475fef85d89a663 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:32:42 +0100 Subject: [PATCH 08/18] Fix merge --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b394484d..531b4185 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,6 @@ serde = { version = "1.0.123", default-features = false, features = ["derive"] } serde_json = "1.0.62" tempfile = "3.2.0" url = { version = "2.2.0", features = ["serde"] } -binaryen = "0.12.0" binaryen = { version = "0.12.0", optional = true } impl-serde = "0.3.1" -- GitLab From 5b2aed30bec4f993c87d0119037ac693fc07ab57 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:38:51 +0100 Subject: [PATCH 09/18] Improve code structure --- src/validation/validate_wasm.rs | 81 ++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/src/validation/validate_wasm.rs b/src/validation/validate_wasm.rs index 8c2f88ac..bc78cb84 100644 --- a/src/validation/validate_wasm.rs +++ b/src/validation/validate_wasm.rs @@ -88,43 +88,7 @@ pub fn validate_import_section(module: &Module) -> Result<()> { aware of a better workaround until the bug in the compiler is fixed.", )); } else if field.contains(INK_ENFORCE_ERR) { - let encoded = field - .strip_prefix(INK_ENFORCE_ERR) - .expect("must exist"); - let hex = serde_hex::from_hex(&encoded) - .expect("decoding hex failed"); - let decoded = ::decode(&mut &hex[..]) - .expect("decoding object failed"); - match decoded { - EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { - let receiver = match message_mut { - true => "&mut self", - false => "&self", - }; - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ - Please check if the receiver of the function to call is consistent\n\ - with the scope in which it is called. The receiver is `{}`.", - trait_ident, - message_ident, - serde_hex::to_hex(&codec::Encode::encode(&message_selector), false), - receiver - )); - }, - EnforcedErrors::CannotCallTraitConstructor { - trait_ident, constructor_ident, constructor_selector - } => { - errs.push(format!( - "An error was found while compiling the contract:\n\ - The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ - Constructor never need to be forwarded, please check if this is the case.", - trait_ident, - constructor_ident, - serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) - )); - } - } + errs.push(parse_linker_error(field)); } // the only allowed imports @@ -143,6 +107,49 @@ pub fn validate_import_section(module: &Module) -> Result<()> { Ok(()) } +/// Extracts the ink! linker error marker from the `field`, parses it, and +/// returns a human readable error message for it. +fn parse_linker_error(field: &str) -> String { + let encoded = field + .strip_prefix(INK_ENFORCE_ERR) + .expect("error marker must exist as prefix"); + let hex = serde_hex::from_hex(&encoded) + .expect("decoding hex failed"); + let decoded = ::decode(&mut &hex[..]) + .expect("decoding object failed"); + + match decoded { + EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { + let receiver = match message_mut { + true => "&mut self", + false => "&self", + }; + format!( + "An error was found while compiling the contract:\n\ + The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ + Please check if the receiver of the function to call is consistent\n\ + with the scope in which it is called. The receiver is `{}`.", + trait_ident, + message_ident, + serde_hex::to_hex(&codec::Encode::encode(&message_selector), false), + receiver + ) + }, + EnforcedErrors::CannotCallTraitConstructor { + trait_ident, constructor_ident, constructor_selector + } => { + format!( + "An error was found while compiling the contract:\n\ + The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ + Constructor never need to be forwarded, please check if this is the case.", + trait_ident, + constructor_ident, + serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) + ) + } + } +} + #[cfg(test)] mod tests { use super::validate_import_section; -- GitLab From f31721aff37b225b2785732e8b903f3a09589ab2 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:40:43 +0100 Subject: [PATCH 10/18] Fix formatting --- src/validation/validate_wasm.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/validation/validate_wasm.rs b/src/validation/validate_wasm.rs index bc78cb84..a55e7fd0 100644 --- a/src/validation/validate_wasm.rs +++ b/src/validation/validate_wasm.rs @@ -113,35 +113,41 @@ fn parse_linker_error(field: &str) -> String { let encoded = field .strip_prefix(INK_ENFORCE_ERR) .expect("error marker must exist as prefix"); - let hex = serde_hex::from_hex(&encoded) - .expect("decoding hex failed"); - let decoded = ::decode(&mut &hex[..]) - .expect("decoding object failed"); + let hex = serde_hex::from_hex(&encoded).expect("decoding hex failed"); + let decoded = + ::decode(&mut &hex[..]).expect("decoding object failed"); match decoded { - EnforcedErrors::CannotCallTraitMessage { trait_ident, message_ident, message_selector, message_mut } => { + EnforcedErrors::CannotCallTraitMessage { + trait_ident, + message_ident, + message_selector, + message_mut, + } => { let receiver = match message_mut { true => "&mut self", false => "&self", }; format!( "An error was found while compiling the contract:\n\ - The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ - Please check if the receiver of the function to call is consistent\n\ - with the scope in which it is called. The receiver is `{}`.", + The ink! message `{}::{}` with the selector `{}` contains an invalid trait call.\n\n\ + Please check if the receiver of the function to call is consistent\n\ + with the scope in which it is called. The receiver is `{}`.", trait_ident, message_ident, serde_hex::to_hex(&codec::Encode::encode(&message_selector), false), receiver ) - }, + } EnforcedErrors::CannotCallTraitConstructor { - trait_ident, constructor_ident, constructor_selector + trait_ident, + constructor_ident, + constructor_selector, } => { format!( "An error was found while compiling the contract:\n\ - The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ - Constructor never need to be forwarded, please check if this is the case.", + The ink! constructor `{}::{}` with the selector `{}` contains an invalid trait call.\n\ + Constructor never need to be forwarded, please check if this is the case.", trait_ident, constructor_ident, serde_hex::to_hex(&codec::Encode::encode(&constructor_selector), false) -- GitLab From 1283115135f21264629acb24e2aed4b64e40d856 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 08:43:22 +0100 Subject: [PATCH 11/18] Improve code structure --- src/validation/validate_wasm.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/validation/validate_wasm.rs b/src/validation/validate_wasm.rs index a55e7fd0..ab067ee9 100644 --- a/src/validation/validate_wasm.rs +++ b/src/validation/validate_wasm.rs @@ -91,8 +91,7 @@ pub fn validate_import_section(module: &Module) -> Result<()> { errs.push(parse_linker_error(field)); } - // the only allowed imports - field.contains("seal") | field.contains("memory") + is_import_allowed(field) }); if original_imports_len != filtered_imports.count() { @@ -107,6 +106,11 @@ pub fn validate_import_section(module: &Module) -> Result<()> { Ok(()) } +/// Returns `true` if the import is allowed. +fn is_import_allowed(field: &str) -> bool { + field.contains("seal") | field.contains("memory") +} + /// Extracts the ink! linker error marker from the `field`, parses it, and /// returns a human readable error message for it. fn parse_linker_error(field: &str) -> String { -- GitLab From 15f155b1690dc756533801168b886577dcdb515f Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:04:00 +0100 Subject: [PATCH 12/18] Remove mod indirection --- src/cmd/build.rs | 3 +-- src/main.rs | 2 +- src/{validation => }/validate_wasm.rs | 0 src/validation/mod.rs | 17 ----------------- 4 files changed, 2 insertions(+), 20 deletions(-) rename src/{validation => }/validate_wasm.rs (100%) delete mode 100644 src/validation/mod.rs diff --git a/src/cmd/build.rs b/src/cmd/build.rs index b3a53a52..98060919 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -26,8 +26,7 @@ use std::{io, process::Command}; use crate::{ crate_metadata::CrateMetadata, - maybe_println, util, - validation::validate_wasm, + maybe_println, util, validate_wasm, workspace::{ManifestPath, Profile, Workspace}, BuildArtifacts, BuildResult, UnstableFlags, UnstableOptions, VerbosityFlags, }; diff --git a/src/main.rs b/src/main.rs index c178d84c..0d4c63e5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,7 @@ mod cmd; mod crate_metadata; mod util; -mod validation; +mod validate_wasm; mod workspace; use self::workspace::ManifestPath; diff --git a/src/validation/validate_wasm.rs b/src/validate_wasm.rs similarity index 100% rename from src/validation/validate_wasm.rs rename to src/validate_wasm.rs diff --git a/src/validation/mod.rs b/src/validation/mod.rs deleted file mode 100644 index 7846f72d..00000000 --- a/src/validation/mod.rs +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2018-2021 Parity Technologies (UK) Ltd. -// This file is part of cargo-contract. -// -// cargo-contract is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// cargo-contract is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with cargo-contract. If not, see . - -pub mod validate_wasm; -- GitLab From e13caccc90168f00a62bf5cd98b4000a7a179a0e Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:09:22 +0100 Subject: [PATCH 13/18] Replace `contains` with `starts_with` --- src/validate_wasm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index ab067ee9..7fff2cdc 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -87,7 +87,7 @@ pub fn validate_import_section(module: &Module) -> Result<()> { This will disable safe math operations, but unfortunately we are currently not \n\ aware of a better workaround until the bug in the compiler is fixed.", )); - } else if field.contains(INK_ENFORCE_ERR) { + } else if field.starts_with(INK_ENFORCE_ERR) { errs.push(parse_linker_error(field)); } @@ -108,7 +108,7 @@ pub fn validate_import_section(module: &Module) -> Result<()> { /// Returns `true` if the import is allowed. fn is_import_allowed(field: &str) -> bool { - field.contains("seal") | field.contains("memory") + field.starts_with("seal") | field.starts_with("memory") } /// Extracts the ink! linker error marker from the `field`, parses it, and -- GitLab From 2a66c7984e10ec53acc14982a800f21e7eb3bb3e Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:15:43 +0100 Subject: [PATCH 14/18] Add note regarding equivalent ink! type necessity --- src/validate_wasm.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index 7fff2cdc..4393d1b7 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -30,6 +30,13 @@ const INK_ENFORCE_ERR: &str = "__ink_enforce_error_"; /// of the contract. /// `cargo-contract` checks the contract code for these error markers /// when building a contract and fails if it finds markers. +/// +/// # Important Note +/// +/// This is a copy of the equivalent type in ink!, which currently resides +/// [here](https://github.com/paritytech/ink/blob/master/crates/lang/codegen/src/generator/cross_calling.rs). +/// This type must be compatible with the ink! version in order to decode +/// the error encoded in the marker. #[derive(codec::Encode, codec::Decode)] pub enum EnforcedErrors { /// The below error represents calling a `&mut self` message in a context that -- GitLab From 7a93454c7fb20274eebdccac6d012eb4506e6fcc Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:31:32 +0100 Subject: [PATCH 15/18] Throw an error on invalid import function --- src/validate_wasm.rs | 49 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index 4393d1b7..63be4540 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -98,7 +98,13 @@ pub fn validate_import_section(module: &Module) -> Result<()> { errs.push(parse_linker_error(field)); } - is_import_allowed(field) + match check_import(field) { + Ok(_) => true, + Err(err) => { + errs.push(err); + false + } + } }); if original_imports_len != filtered_imports.count() { @@ -114,8 +120,21 @@ pub fn validate_import_section(module: &Module) -> Result<()> { } /// Returns `true` if the import is allowed. -fn is_import_allowed(field: &str) -> bool { - field.starts_with("seal") | field.starts_with("memory") +fn check_import(field: &str) -> Result<(), String> { + let allowed_prefixes = ["seal", "memory"]; + if allowed_prefixes + .iter() + .any(|prefix| field.starts_with(prefix)) + { + Ok(()) + } else { + let msg = format!( + "An unexpected import function was found in the contract Wasm: {}.\n\ + The only allowed import functions are those starting with one of the following prefixes:\n\ + {}", field, allowed_prefixes.join(", ") + ); + Err(msg) + } } /// Extracts the ink! linker error marker from the `field`, parses it, and @@ -178,7 +197,7 @@ mod tests { } #[test] - fn validate_must_catch_panic_import() { + fn must_catch_panic_import() { // given let contract = r#" (module @@ -241,4 +260,26 @@ mod tests { "The ink! constructor `Flip::new` with the selector `0x40d75d74` contains an invalid trait call." )); } + + #[test] + fn must_catch_invalid_import() { + // given + let contract = r#" + (module + (type (;0;) (func (param i32 i32 i32))) + (import "env" "some_fn" (func (;5;) (type 0))) + (func (;5;) (type 0)) + )"#; + let module = create_module(contract); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_err()); + assert!(res + .unwrap_err() + .to_string() + .contains("An unexpected import function was found in the contract Wasm: some_fn.")); + } } -- GitLab From 7d0c7e0f57fe56d74a95c3ef71a7c6873cf1aacf Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:36:56 +0100 Subject: [PATCH 16/18] Add sunny day test --- src/validate_wasm.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index 63be4540..3093a308 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -282,4 +282,23 @@ mod tests { .to_string() .contains("An unexpected import function was found in the contract Wasm: some_fn.")); } + + #[test] + fn must_validate_successfully() { + // given + let contract = r#" + (module + (type (;0;) (func (param i32 i32 i32))) + (import "env" "seal_foo" (func (;5;) (type 0))) + (import "env" "memory" (func (;5;) (type 0))) + (func (;5;) (type 0)) + )"#; + let module = create_module(contract); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_ok()); + } } -- GitLab From 2b292e1308b4153e13c40ac9ec1cc93fd628b4d2 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 14:41:50 +0100 Subject: [PATCH 17/18] Do not panic if no import section is found --- src/validate_wasm.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index 3093a308..958752bd 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -75,11 +75,14 @@ pub enum EnforcedErrors { /// - Known bugs for which we want to recommend a solution. /// - Markers inserted by the ink! codegen for errors which can't be checked at compile time. pub fn validate_import_section(module: &Module) -> Result<()> { - let imports = module - .import_section() - .expect("import section must exist") - .entries() - .iter(); + let imports = match module.import_section() { + Some(section) => section.entries().iter(), + None => { + // the module does not contain any imports, + // hence no further validation is necessary. + return Ok(()); + } + }; let original_imports_len = imports.len(); let mut errs = Vec::new(); @@ -301,4 +304,17 @@ mod tests { // then assert!(res.is_ok()); } + + #[test] + fn must_validate_successfully_if_no_import_section_found() { + // given + let contract = r#"(module)"#; + let module = create_module(contract); + + // when + let res = validate_import_section(&module); + + // then + assert!(res.is_ok()); + } } -- GitLab From f5851118517f0335f2415d609c46ed2333be5e3f Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 11 Feb 2021 15:29:53 +0100 Subject: [PATCH 18/18] Update error message on decoding error --- src/validate_wasm.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/validate_wasm.rs b/src/validate_wasm.rs index 958752bd..66fc6ff2 100644 --- a/src/validate_wasm.rs +++ b/src/validate_wasm.rs @@ -147,8 +147,11 @@ fn parse_linker_error(field: &str) -> String { .strip_prefix(INK_ENFORCE_ERR) .expect("error marker must exist as prefix"); let hex = serde_hex::from_hex(&encoded).expect("decoding hex failed"); - let decoded = - ::decode(&mut &hex[..]).expect("decoding object failed"); + let decoded = ::decode(&mut &hex[..]).expect( + "The `EnforcedError` object could not be decoded. The probable\ + cause is a mismatch between the ink! definition of the type and the\ + local `cargo-contract` definition.", + ); match decoded { EnforcedErrors::CannotCallTraitMessage { -- GitLab