Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pallet-revive: Statically verify imports on code deployment #6759

Merged
merged 12 commits into from
Dec 11, 2024
2 changes: 1 addition & 1 deletion substrate/frame/revive/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Driven by the desire to have an iterative approach in developing new contract in
concept of an unstable interface. Akin to the rust nightly compiler it allows us to add new interfaces but mark them as
unstable so that contract languages can experiment with them and give feedback before we stabilize those.

In order to access interfaces which don't have a stable `#[api_version(x)]` in [`runtime.rs`](src/wasm/runtime.rs)
In order to access interfaces which don't have a stable `#[stable]` in [`runtime.rs`](src/wasm/runtime.rs)
one need to set `pallet_revive::Config::UnsafeUnstableInterface` to `ConstU32<true>`.
**It should be obvious that any production runtime should never be compiled with this feature: In addition to be
subject to change or removal those interfaces might not have proper weights associated with them and are therefore
Expand Down
44 changes: 44 additions & 0 deletions substrate/frame/revive/fixtures/contracts/unknown_syscall.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#![no_std]
#![no_main]

extern crate common;

#[polkavm_derive::polkavm_import]
extern "C" {
pub fn __this_syscall_does_not_exist__();
}

// Export that is never called. We can put code here that should be in the binary
// but is never supposed to be run.
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call_never() {
// make sure it is not optimized away
unsafe {
__this_syscall_does_not_exist__();
}
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {}
44 changes: 44 additions & 0 deletions substrate/frame/revive/fixtures/contracts/unstable_interface.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#![no_std]
#![no_main]

extern crate common;

#[polkavm_derive::polkavm_import]
extern "C" {
pub fn set_code_hash();
}
Comment on lines +22 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we make a dedicated unstable api just for test so that this does not fail when this becomes stable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that is a good idea. But we will cross the bridge once we get there.


// Export that is never called. We can put code here that should be in the binary
// but is never supposed to be run.
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call_never() {
// make sure it is not optimized away
unsafe {
set_code_hash();
}
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {}
72 changes: 37 additions & 35 deletions substrate/frame/revive/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct EnvDef {
/// Parsed host function definition.
struct HostFn {
item: syn::ItemFn,
api_version: Option<u16>,
is_stable: bool,
name: String,
returns: HostFnReturn,
cfg: Option<syn::Attribute>,
Expand Down Expand Up @@ -183,22 +183,21 @@ impl HostFn {
};

// process attributes
let msg = "Only #[api_version(<u16>)], #[cfg] and #[mutating] attributes are allowed.";
let msg = "Only #[stable], #[cfg] and #[mutating] attributes are allowed.";
let span = item.span();
let mut attrs = item.attrs.clone();
attrs.retain(|a| !a.path().is_ident("doc"));
let mut api_version = None;
let mut is_stable = false;
let mut mutating = false;
let mut cfg = None;
while let Some(attr) = attrs.pop() {
let ident = attr.path().get_ident().ok_or(err(span, msg))?.to_string();
match ident.as_str() {
"api_version" => {
if api_version.is_some() {
return Err(err(span, "#[api_version] can only be specified once"))
"stable" => {
if is_stable {
return Err(err(span, "#[stable] can only be specified once"))
}
api_version =
Some(attr.parse_args::<syn::LitInt>().and_then(|lit| lit.base10_parse())?);
is_stable = true;
},
"mutating" => {
if mutating {
Expand Down Expand Up @@ -313,7 +312,7 @@ impl HostFn {
_ => Err(err(arg1.span(), &msg)),
}?;

Ok(Self { item, api_version, name, returns, cfg })
Ok(Self { item, is_stable, name, returns, cfg })
},
_ => Err(err(span, &msg)),
}
Expand Down Expand Up @@ -411,19 +410,23 @@ fn expand_env(def: &EnvDef) -> TokenStream2 {
let impls = expand_functions(def);
let bench_impls = expand_bench_functions(def);
let docs = expand_func_doc(def);
let highest_api_version =
def.host_funcs.iter().filter_map(|f| f.api_version).max().unwrap_or_default();
let stable_syscalls = expand_func_list(def, false);
let all_syscalls = expand_func_list(def, true);

quote! {
#[cfg(test)]
pub const HIGHEST_API_VERSION: u16 = #highest_api_version;
pub fn list_syscalls(include_unstable: bool) -> &'static [&'static [u8]] {
if include_unstable {
#all_syscalls
} else {
#stable_syscalls
}
}

impl<'a, E: Ext, M: PolkaVmInstance<E::T>> Runtime<'a, E, M> {
fn handle_ecall(
&mut self,
memory: &mut M,
__syscall_symbol__: &[u8],
__available_api_version__: ApiVersion,
) -> Result<Option<u64>, TrapReason>
{
#impls
Expand Down Expand Up @@ -474,10 +477,6 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {
let body = &f.item.block;
let map_output = f.returns.map_output();
let output = &f.item.sig.output;
let api_version = match f.api_version {
Some(version) => quote! { Some(#version) },
None => quote! { None },
};

// wrapped host function body call with host function traces
// see https://github.com/paritytech/polkadot-sdk/tree/master/substrate/frame/contracts#host-function-tracing
Expand Down Expand Up @@ -513,7 +512,7 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {

quote! {
#cfg
#syscall_symbol if __is_available__(#api_version) => {
#syscall_symbol => {
// closure is needed so that "?" can infere the correct type
(|| #output {
#arg_decoder
Expand All @@ -534,18 +533,6 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {
// This is the overhead to call an empty syscall that always needs to be charged.
self.charge_gas(crate::wasm::RuntimeCosts::HostFn).map_err(TrapReason::from)?;

// Not all APIs are available depending on configuration or when the code was deployed.
// This closure will be used by syscall specific code to perform this check.
let __is_available__ = |syscall_version: Option<u16>| {
match __available_api_version__ {
ApiVersion::UnsafeNewest => true,
ApiVersion::Versioned(max_available_version) =>
syscall_version
.map(|required_version| max_available_version >= required_version)
.unwrap_or(false),
}
};

// They will be mapped to variable names by the syscall specific code.
let (__a0__, __a1__, __a2__, __a3__, __a4__, __a5__) = memory.read_input_regs();

Expand Down Expand Up @@ -607,10 +594,8 @@ fn expand_func_doc(def: &EnvDef) -> TokenStream2 {
});
quote! { #( #docs )* }
};
let availability = if let Some(version) = func.api_version {
let info = format!(
"\n# Required API version\nThis API was added in version **{version}**.",
);
let availability = if func.is_stable {
let info = format!("\n# Stable API\nThis API is stable and will never change.",);
quote! { #[doc = #info] }
} else {
let info =
Expand All @@ -632,3 +617,20 @@ fn expand_func_doc(def: &EnvDef) -> TokenStream2 {
#( #docs )*
}
}

fn expand_func_list(def: &EnvDef, include_unstable: bool) -> TokenStream2 {
let docs = def.host_funcs.iter().filter(|f| include_unstable || f.is_stable).map(|f| {
let name = Literal::byte_string(f.name.as_bytes());
quote! {
#name.as_slice()
}
});
let len = docs.clone().count();

quote! {
{
static FUNCS: [&[u8]; #len] = [#(#docs),*];
FUNCS.as_slice()
}
}
}
10 changes: 2 additions & 8 deletions substrate/frame/revive/src/benchmarking/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
exec::{ExportedFunction, Ext, Key, Stack},
storage::meter::Meter,
transient_storage::MeterEntry,
wasm::{ApiVersion, PreparedCall, Runtime},
wasm::{PreparedCall, Runtime},
BalanceOf, Config, DebugBuffer, Error, GasMeter, MomentOf, Origin, WasmBlob, Weight,
};
use alloc::{vec, vec::Vec};
Expand Down Expand Up @@ -164,13 +164,7 @@ where
module: WasmBlob<T>,
input: Vec<u8>,
) -> PreparedCall<'a, StackExt<'a, T>> {
module
.prepare_call(
Runtime::new(ext, input),
ExportedFunction::Call,
ApiVersion::UnsafeNewest,
)
.unwrap()
module.prepare_call(Runtime::new(ext, input), ExportedFunction::Call).unwrap()
}

/// Add transient_storage
Expand Down
21 changes: 0 additions & 21 deletions substrate/frame/revive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,6 @@ const SENTINEL: u32 = u32::MAX;
/// Example: `RUST_LOG=runtime::revive=debug my_code --dev`
const LOG_TARGET: &str = "runtime::revive";

/// This version determines which syscalls are available to contracts.
///
/// Needs to be bumped every time a versioned syscall is added.
const API_VERSION: u16 = 0;

#[test]
fn api_version_up_to_date() {
assert!(
API_VERSION == crate::wasm::HIGHEST_API_VERSION,
"A new versioned API has been added. The `API_VERSION` needs to be bumped."
);
}

#[frame_support::pallet]
pub mod pallet {
use super::*;
Expand Down Expand Up @@ -623,14 +610,6 @@ pub mod pallet {
#[pallet::storage]
pub(crate) type AddressSuffix<T: Config> = StorageMap<_, Identity, H160, [u8; 12]>;

#[pallet::extra_constants]
impl<T: Config> Pallet<T> {
#[pallet::constant_name(ApiVersion)]
fn api_version() -> u16 {
API_VERSION
}
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_idle(_block: BlockNumberFor<T>, limit: Weight) -> Weight {
Expand Down
25 changes: 24 additions & 1 deletion substrate/frame/revive/src/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ pub mod code {
const BASIC_BLOCK_SIZE: u32 = 1000;

/// Make sure that the various program parts are within the defined limits.
pub fn enforce<T: Config>(blob: Vec<u8>) -> Result<CodeVec, DispatchError> {
pub fn enforce<T: Config>(
blob: Vec<u8>,
available_syscalls: &[&[u8]],
) -> Result<CodeVec, DispatchError> {
fn round_page(n: u32) -> u64 {
// performing the rounding in u64 in order to prevent overflow
u64::from(n).next_multiple_of(PAGE_SIZE.into())
Expand All @@ -134,6 +137,26 @@ pub mod code {
Err(Error::<T>::CodeRejected)?;
}

// Need to check that no non-existent syscalls are used. This allows us to add
// new syscalls later without affecting already deployed code.
for (idx, import) in program.imports().iter().enumerate() {
// We are being defensive in case an attacker is able to somehow include
// a lot of imports. This is important because we search the array of host
// functions for every import.
if idx == available_syscalls.len() {
log::debug!(target: LOG_TARGET, "Program contains too many imports.");
Err(Error::<T>::CodeRejected)?;
}
let Some(import) = import else {
log::debug!(target: LOG_TARGET, "Program contains malformed import.");
return Err(Error::<T>::CodeRejected.into());
};
if !available_syscalls.contains(&import.as_bytes()) {
log::debug!(target: LOG_TARGET, "Program references unknown syscall: {}", import);
Err(Error::<T>::CodeRejected)?;
}
}

// This scans the whole program but we only do it once on code deployment.
// It is safe to do unchecked math in u32 because the size of the program
// was already checked above.
Expand Down
32 changes: 32 additions & 0 deletions substrate/frame/revive/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4666,3 +4666,35 @@ fn mapped_address_works() {
assert_eq!(<Test as Config>::Currency::total_balance(&EVE), 1_100);
});
}

#[test]
fn unknown_syscall_rejected() {
let (code, _) = compile_module("unknown_syscall").unwrap();

ExtBuilder::default().existential_deposit(100).build().execute_with(|| {
<Test as Config>::Currency::set_balance(&ALICE, 1_000_000);

assert_err!(
builder::bare_instantiate(Code::Upload(code)).build().result,
<Error<Test>>::CodeRejected,
)
});
}

#[test]
fn unstable_interface_rejected() {
let (code, _) = compile_module("unstable_interface").unwrap();

ExtBuilder::default().existential_deposit(100).build().execute_with(|| {
<Test as Config>::Currency::set_balance(&ALICE, 1_000_000);

Test::set_unstable_interface(false);
assert_err!(
builder::bare_instantiate(Code::Upload(code.clone())).build().result,
<Error<Test>>::CodeRejected,
);

Test::set_unstable_interface(true);
assert_ok!(builder::bare_instantiate(Code::Upload(code)).build().result);
});
}
Loading
Loading