From 07268022cc5ef6c8b104d154c742012f6934fe4a Mon Sep 17 00:00:00 2001
From: Sergei Pepyakin <s.pepyakin@gmail.com>
Date: Fri, 19 Apr 2019 19:35:11 +0200
Subject: [PATCH] contracts: Validate code before deployment (#2330)
* Validate module before storing it in code_cache.
* Bump version.
---
substrate/Cargo.lock | 11 +++++++
substrate/node/runtime/Cargo.toml | 3 ++
substrate/node/runtime/src/lib.rs | 2 +-
substrate/node/runtime/wasm/Cargo.lock | 10 ++++++
substrate/node/runtime/wasm/Cargo.toml | 5 ++-
substrate/srml/contract/Cargo.toml | 5 +++
.../srml/contract/src/wasm/code_cache.rs | 2 --
substrate/srml/contract/src/wasm/prepare.rs | 31 ++++++++++++++-----
8 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index be655691971..5e35d98feec 100644
--- a/substrate/Cargo.lock
+++ b/substrate/Cargo.lock
@@ -3233,6 +3233,7 @@ dependencies = [
"srml-timestamp 1.0.0",
"substrate-primitives 1.0.0",
"wabt 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)",
+ "wasmi-validation 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
@@ -5071,6 +5072,15 @@ dependencies = [
"parity-wasm 0.31.3 (registry+https://github.com/rust-lang/crates.io-index)",
]
+[[package]]
+name = "wasmi-validation"
+version = "0.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "hashbrown 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-wasm 0.31.3 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
[[package]]
name = "websocket"
version = "0.22.2"
@@ -5603,6 +5613,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9d9d7ed3431229a144296213105a390676cc49c9b6a72bd19f3176c98e129fa1"
"checksum want 0.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "797464475f30ddb8830cc529aaaae648d581f99e2036a928877dfde027ddf6b3"
"checksum wasmi 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "21ef487a11df1ed468cf613c78798c26282da5c30e9d49f824872d4c77b47d1d"
+"checksum wasmi-validation 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ab380192444b3e8522ae79c0a1976e42a82920916ccdfbce3def89f456ea33f3"
"checksum websocket 0.22.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d2c67346c042adbd4f5b2a49700e340befc5b772094fec8d36df6b825523d933"
"checksum which 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)" = "e84a603e7e0b1ce1aa1ee2b109c7be00155ce52df5081590d1ffb93f4f515cb2"
"checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a"
diff --git a/substrate/node/runtime/Cargo.toml b/substrate/node/runtime/Cargo.toml
index 9cdceb96ac9..f8e0e047bc9 100644
--- a/substrate/node/runtime/Cargo.toml
+++ b/substrate/node/runtime/Cargo.toml
@@ -41,6 +41,9 @@ consensus_authorities = { package = "substrate-consensus-authorities", path = ".
[features]
default = ["std"]
+core = [
+ "contract/core",
+]
std = [
"parity-codec/std",
"substrate-primitives/std",
diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs
index 7239e14aa1b..3a3ad41c289 100644
--- a/substrate/node/runtime/src/lib.rs
+++ b/substrate/node/runtime/src/lib.rs
@@ -59,7 +59,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
authoring_version: 10,
- spec_version: 62,
+ spec_version: 63,
impl_version: 65,
apis: RUNTIME_API_VERSIONS,
};
diff --git a/substrate/node/runtime/wasm/Cargo.lock b/substrate/node/runtime/wasm/Cargo.lock
index af458157776..243a60fb254 100644
--- a/substrate/node/runtime/wasm/Cargo.lock
+++ b/substrate/node/runtime/wasm/Cargo.lock
@@ -2226,6 +2226,7 @@ dependencies = [
"srml-system 1.0.0",
"srml-timestamp 1.0.0",
"substrate-primitives 1.0.0",
+ "wasmi-validation 0.1.0",
]
[[package]]
@@ -3203,6 +3204,15 @@ dependencies = [
"parity-wasm 0.31.3 (registry+https://github.com/rust-lang/crates.io-index)",
]
+[[package]]
+name = "wasmi-validation"
+version = "0.1.0"
+dependencies = [
+ "hashbrown 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "memory_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-wasm 0.31.3 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
[[package]]
name = "winapi"
version = "0.2.8"
diff --git a/substrate/node/runtime/wasm/Cargo.toml b/substrate/node/runtime/wasm/Cargo.toml
index 8fd90890e95..b6490a047ed 100644
--- a/substrate/node/runtime/wasm/Cargo.toml
+++ b/substrate/node/runtime/wasm/Cargo.toml
@@ -12,7 +12,10 @@ crate-type = ["cdylib"]
node-runtime = { path = "..", default-features = false }
[features]
-default = []
+default = ["core"]
+core = [
+ "node-runtime/core",
+]
std = [
"node-runtime/std",
]
diff --git a/substrate/srml/contract/Cargo.toml b/substrate/srml/contract/Cargo.toml
index 2338aabe0d2..be4034c849e 100644
--- a/substrate/srml/contract/Cargo.toml
+++ b/substrate/srml/contract/Cargo.toml
@@ -10,6 +10,7 @@ serde_derive = { version = "1.0", optional = true }
pwasm-utils = { version = "0.6.1", default-features = false }
parity-codec = { version = "3.3", default-features = false, features = ["derive"] }
parity-wasm = { version = "0.31", default-features = false }
+wasmi-validation = { version = "0.1", default-features = false }
substrate-primitives = { path = "../../core/primitives", default-features = false }
runtime-primitives = { package = "sr-primitives", path = "../../core/sr-primitives", default-features = false }
runtime-io = { package = "sr-io", path = "../../core/sr-io", default-features = false }
@@ -28,6 +29,9 @@ balances = { package = "srml-balances", path = "../balances" }
[features]
default = ["std"]
+core = [
+ "wasmi-validation/core",
+]
std = [
"serde",
"serde_derive",
@@ -42,4 +46,5 @@ std = [
"timestamp/std",
"parity-wasm/std",
"pwasm-utils/std",
+ "wasmi-validation/std",
]
diff --git a/substrate/srml/contract/src/wasm/code_cache.rs b/substrate/srml/contract/src/wasm/code_cache.rs
index dab8c4bfa4b..0c71fe8cb5b 100644
--- a/substrate/srml/contract/src/wasm/code_cache.rs
+++ b/substrate/srml/contract/src/wasm/code_cache.rs
@@ -72,8 +72,6 @@ pub fn save<T: Trait>(
let prefab_module = prepare::prepare_contract::<T, Env>(&original_code, schedule)?;
let code_hash = T::Hashing::hash(&original_code);
- // TODO: #1416 validate the code. If the code is not valid, then don't store it.
-
<CodeStorage<T>>::insert(code_hash, prefab_module);
<PristineCode<T>>::insert(code_hash, original_code);
diff --git a/substrate/srml/contract/src/wasm/prepare.rs b/substrate/srml/contract/src/wasm/prepare.rs
index 15922c3d999..53883451b42 100644
--- a/substrate/srml/contract/src/wasm/prepare.rs
+++ b/substrate/srml/contract/src/wasm/prepare.rs
@@ -29,20 +29,34 @@ use rstd::prelude::*;
use runtime_primitives::traits::As;
struct ContractModule<'a, Gas: 'a> {
- // An `Option` is used here for loaning (`take()`-ing) the module.
- // Invariant: Can't be `None` (i.e. on enter and on exit from the function
- // the value *must* be `Some`).
+ /// A deserialized module. The module is valid (this is Guaranteed by `new` method).
+ ///
+ /// An `Option` is used here for loaning (`take()`-ing) the module.
+ /// Invariant: Can't be `None` (i.e. on enter and on exit from the function
+ /// the value *must* be `Some`).
module: Option<elements::Module>,
schedule: &'a Schedule<Gas>,
}
impl<'a, Gas: 'a + As<u32> + Clone> ContractModule<'a, Gas> {
+ /// Creates a new instance of `ContractModule`.
+ ///
+ /// Returns `Err` if the `original_code` couldn't be decoded or
+ /// if it contains an invalid module.
fn new(
original_code: &[u8],
schedule: &'a Schedule<Gas>,
) -> Result<ContractModule<'a, Gas>, &'static str> {
+ use wasmi_validation::{validate_module, PlainValidator};
+
let module =
- elements::deserialize_buffer(original_code).map_err(|_| "can't decode wasm code")?;
+ elements::deserialize_buffer(original_code).map_err(|_| "Can't decode wasm code")?;
+
+ // Make sure that the module is valid.
+ validate_module::<PlainValidator>(&module).map_err(|_| "Module is not valid")?;
+
+ // Return a `ContractModule` instance with
+ // __valid__ module.
Ok(ContractModule {
module: Some(module),
schedule,
@@ -270,7 +284,8 @@ impl<'a, Gas: 'a + As<u32> + Clone> ContractModule<'a, Gas> {
///
/// The checks are:
///
-/// - module doesn't define an internal memory instance,
+/// - provided code is a valid wasm module.
+/// - the module doesn't define an internal memory instance,
/// - imported memory (if any) doesn't reserve more memory than permitted by the `schedule`,
/// - all imported functions from the external environment matches defined by `env` module,
///
@@ -438,7 +453,7 @@ mod tests {
(func (export "deploy"))
)
"#,
- Err("Requested initial number of pages should not exceed the requested maximum")
+ Err("Module is not valid")
);
prepare_test!(no_maximum,
@@ -487,7 +502,7 @@ mod tests {
(func (export "deploy"))
)
"#,
- Err("Multiple memory imports defined")
+ Err("Module is not valid")
);
prepare_test!(table_import,
@@ -505,7 +520,7 @@ mod tests {
prepare_test!(global_import,
r#"
(module
- (global $g (import "env" "global") (mut i32))
+ (global $g (import "env" "global") i32)
(func (export "call"))
(func (export "deploy"))
)
--
GitLab