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

Support security fixes #64

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,630 changes: 1,522 additions & 1,108 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions language/move-model/src/builder/model_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub(crate) struct StructEntry {
#[derive(Debug, Clone)]
pub(crate) struct FunEntry {
pub loc: Loc,
pub id_loc: Loc,
pub module_id: ModuleId,
pub fun_id: FunId,
pub visibility: FunctionVisibility,
Expand Down Expand Up @@ -269,6 +270,7 @@ impl<'env> ModelBuilder<'env> {
pub fn define_fun(
&mut self,
loc: Loc,
id_loc: Loc,
attributes: Vec<Attribute>,
name: QualifiedSymbol,
module_id: ModuleId,
Expand All @@ -281,6 +283,7 @@ impl<'env> ModelBuilder<'env> {
) {
let entry = FunEntry {
loc,
id_loc,
attributes,
module_id,
fun_id,
Expand Down
4 changes: 4 additions & 0 deletions language/move-model/src/builder/module_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use move_compiler::{
parser::ast as PA,
shared::{unique_map::UniqueMap, Name},
};
use move_compiler::shared::Identifier;
use move_ir_types::{ast::ConstantName, location::Spanned};

use crate::{
Expand Down Expand Up @@ -427,8 +428,10 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
EA::Visibility::Internal => FunctionVisibility::Private,
};
let loc = et.to_loc(&def.loc);
let id_loc = et.to_loc(&name.loc());
et.parent.parent.define_fun(
loc.clone(),
id_loc,
attrs,
qsym.clone(),
et.parent.module_id,
Expand Down Expand Up @@ -3191,6 +3194,7 @@ impl<'env, 'translator> ModuleBuilder<'env, 'translator> {
def_idx,
name,
entry.loc.clone(),
entry.id_loc.clone(),
entry.attributes.clone(),
arg_names,
type_arg_names,
Expand Down
11 changes: 11 additions & 0 deletions language/move-model/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,7 @@ impl GlobalEnv {
def_idx: FunctionDefinitionIndex,
name: Symbol,
loc: Loc,
id_loc: Loc,
attributes: Vec<Attribute>,
arg_names: Vec<Symbol>,
type_arg_names: Vec<Symbol>,
Expand All @@ -1178,6 +1179,7 @@ impl GlobalEnv {
FunctionData {
name,
loc,
id_loc,
attributes,
def_idx,
handle_idx,
Expand Down Expand Up @@ -2925,6 +2927,9 @@ pub struct FunctionData {
/// Location of this function.
loc: Loc,

/// Location of the function identifier, suitable for error messages alluding to the function.
id_loc: Loc,

/// The definition index of this function in its module.
def_idx: FunctionDefinitionIndex,

Expand Down Expand Up @@ -2963,6 +2968,7 @@ impl FunctionData {
FunctionData {
name,
loc: Loc::default(),
id_loc: Loc::default(),
attributes: Vec::default(),
def_idx,
handle_idx,
Expand Down Expand Up @@ -3037,6 +3043,11 @@ impl<'env> FunctionEnv<'env> {
self.data.loc.clone()
}

/// Returns the location of this function identifier.
pub fn get_id_loc(&self) -> Loc {
self.data.id_loc.clone()
}

/// Returns the attributes of this function.
pub fn get_attributes(&self) -> &[Attribute] {
&self.data.attributes
Expand Down
1 change: 1 addition & 0 deletions language/move-vm/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ move-bytecode-verifier = { path = "../../move-bytecode-verifier" }
move-core-types = { path = "../../move-core/types" }
move-vm-types = { path = "../types" }
move-binary-format = { path = "../../move-binary-format" }
typed-arena = "2.0.2"

[dev-dependencies]
anyhow = "1.0.52"
Expand Down
77 changes: 77 additions & 0 deletions language/move-vm/runtime/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
// SPDX-License-Identifier: Apache-2.0

use crate::loader::Loader;
use std::collections::btree_map;

use move_binary_format::errors::*;
use move_binary_format::file_format::CompiledScript;
use move_binary_format::CompiledModule;
use move_core_types::language_storage::StructTag;
use move_core_types::{
account_address::AccountAddress,
Expand All @@ -21,7 +24,9 @@ use move_vm_types::{
loaded_data::runtime_types::Type,
values::{GlobalValue, Value},
};
use sha3::{Digest, Sha3_256};
use std::collections::btree_map::BTreeMap;
use std::sync::Arc;

pub struct AccountDataCache {
data_map: BTreeMap<StructTag, (MoveTypeLayout, GlobalValue)>,
Expand Down Expand Up @@ -55,6 +60,10 @@ pub(crate) struct TransactionDataCache<'r, 'l, S> {
loader: &'l Loader,
account_map: BTreeMap<AccountAddress, AccountDataCache>,
event_data: Vec<(Vec<u8>, u64, Type, MoveTypeLayout, Value)>,

// Caches to help avoid duplicate deserialization calls.
compiled_scripts: BTreeMap<[u8; 32], Arc<CompiledScript>>,
compiled_modules: BTreeMap<ModuleId, (Arc<CompiledModule>, usize, [u8; 32])>,
}

impl<'r, 'l, S: MoveResolver> TransactionDataCache<'r, 'l, S> {
Expand All @@ -66,6 +75,9 @@ impl<'r, 'l, S: MoveResolver> TransactionDataCache<'r, 'l, S> {
loader,
account_map: BTreeMap::new(),
event_data: vec![],

compiled_scripts: BTreeMap::new(),
compiled_modules: BTreeMap::new(),
}
}

Expand Down Expand Up @@ -154,6 +166,71 @@ impl<'r, 'l, S: MoveResolver> TransactionDataCache<'r, 'l, S> {
}
map.get_mut(k).unwrap()
}

pub(crate) fn load_compiled_script_to_cache(
&mut self,
script_blob: &[u8],
hash_value: [u8; 32],
) -> VMResult<Arc<CompiledScript>> {
let cache = &mut self.compiled_scripts;
match cache.entry(hash_value) {
btree_map::Entry::Occupied(entry) => Ok(entry.get().clone()),
btree_map::Entry::Vacant(entry) => {
let script = match CompiledScript::deserialize(script_blob) {
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));
}
};
Ok(entry.insert(Arc::new(script)).clone())
}
}
}

pub(crate) fn load_compiled_module_to_cache(
&mut self,
id: ModuleId,
allow_loading_failure: bool,
) -> VMResult<(Arc<CompiledModule>, usize, [u8; 32])> {
let cache = &mut self.compiled_modules;
match cache.entry(id) {
btree_map::Entry::Occupied(entry) => Ok(entry.get().clone()),
btree_map::Entry::Vacant(entry) => {
// bytes fetching, allow loading to fail if the flag is set
let bytes = match load_moule_impl(self.remote, &self.account_map, entry.key())
.map_err(|err| err.finish(Location::Undefined))
{
Ok(bytes) => bytes,
Err(err) if allow_loading_failure => return Err(err),
Err(err) => {
return Err(expect_no_verification_errors(err));
}
};

let mut sha3_256 = Sha3_256::new();
sha3_256.update(&bytes);
let hash_value: [u8; 32] = sha3_256.finalize().into();

// for bytes obtained from the data store, they should always deserialize and verify.
// It is an invariant violation if they don't.
let module = CompiledModule::deserialize(&bytes)
.map_err(|err| {
let msg = format!("Deserialization error: {:?}", err);
PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
.with_message(msg)
.finish(Location::Module(entry.key().clone()))
})
.map_err(expect_no_verification_errors)?;

Ok(entry
.insert((Arc::new(module), bytes.len(), hash_value))
.clone())
}
}
}
}

// `DataStore` implementation for the `TransactionDataCache`
Expand Down
7 changes: 3 additions & 4 deletions language/move-vm/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@

pub mod data_cache;
mod interpreter;
mod loader;
pub mod loader;
pub mod logging;
pub mod move_vm;
pub mod native_extensions;
pub mod native_functions;
mod runtime;
pub mod runtime;
pub mod session;
#[macro_use]
mod tracing;
Expand All @@ -27,7 +27,6 @@ pub mod config;
#[cfg(any(debug_assertions, feature = "debugging"))]
mod debug;

pub mod module_traversal;
#[cfg(test)]
mod unit_tests;

pub mod move_vm_adapter;
Loading