[move-vm] Check cross module linking before loading

third_party/move/move-binary-format/src/file_format.rs

@@ -28,7 +28,7 @@
 //! those structs translate to tables and table specifications.
 
 use crate::{
-    access::ModuleAccess,
+    access::{ModuleAccess, ScriptAccess},
     errors::{PartialVMError, PartialVMResult},
     file_format_common,
     internals::ModuleIndex,
@@ -1841,6 +1841,14 @@ pub struct CompiledScript {
 impl CompiledScript {
     /// Returns the index of `main` in case a script is converted to a module.
     pub const MAIN_INDEX: FunctionDefinitionIndex = FunctionDefinitionIndex(0);
+
+    /// Returns the code key of `module_handle`
+    pub fn module_id_for_handle(&self, module_handle: &ModuleHandle) -> ModuleId {
+        ModuleId::new(
+            *self.address_identifier_at(module_handle.address),
+            self.identifier_at(module_handle.name).to_owned(),
+        )
+    }
 }
 
 /// A `CompiledModule` defines the structure of a module which is the unit of published code.

third_party/move/move-vm/runtime/src/loader/mod.rs

@@ -278,9 +278,7 @@ impl ModuleCache {
 
     // `make_type` is the entry point to "translate" a `SignatureToken` to a `Type`
     fn make_type(&self, module: BinaryIndexedView, tok: &SignatureToken) -> PartialVMResult<Type> {
-        make_type_internal(module, tok, &|struct_name, module_id| {
-            self.resolve_struct_by_name(struct_name, module_id)
-        })
+        make_type_internal(module, tok)
     }
 
     // While in the process of loading, and before a `Module` is saved into the cache the loader
@@ -291,34 +289,7 @@ impl ModuleCache {
         module: &CompiledModule,
         tok: &SignatureToken,
     ) -> PartialVMResult<Type> {
-        let self_id = module.self_id();
-        make_type_internal(
-            BinaryIndexedView::Module(module),
-            tok,
-            &|struct_name, module_id| {
-                if module_id == &self_id {
-                    // module has not been published yet, loop through the types
-                    for (idx, struct_type) in self.structs.iter().enumerate().rev() {
-                        if &struct_type.name.module != module_id {
-                            break;
-                        }
-                        if struct_type.name.name.as_ident_str() == struct_name {
-                            return Ok((CachedStructIndex(idx), struct_type.clone()));
-                        }
-                    }
-                    Err(
-                        PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE).with_message(
-                            format!(
-                                "Cannot find {:?}::{:?} in publishing module",
-                                module_id, struct_name
-                            ),
-                        ),
-                    )
-                } else {
-                    self.resolve_struct_by_name(struct_name, module_id)
-                }
-            },
-        )
+        make_type_internal(BinaryIndexedView::Module(module), tok)
     }
 
     // Given a module id, returns whether the module cache has the module or not
@@ -1902,6 +1873,30 @@ impl Module {
                 }
             }
 
+            // validate the correctness of struct handle references.
+            for struct_handle in module.struct_handles() {
+                let module_handle = module.module_handle_at(struct_handle.module);
+                if module_handle != module.self_handle() {
+                    let struct_name = module.identifier_at(struct_handle.name);
+                    let module_handle = module.module_handle_at(struct_handle.module);
+                    let module_id = module.module_id_for_handle(module_handle);
+                    let (_, struct_) = cache.resolve_struct_by_name(struct_name, &module_id)?;
+                    if !struct_handle.abilities.is_subset(struct_.abilities)
+                        || &struct_handle
+                            .type_parameters
+                            .iter()
+                            .map(|ty| ty.is_phantom)
+                            .collect::<Vec<_>>()
+                            != &struct_.phantom_ty_args_mask
+                    {
+                        return Err(PartialVMError::new(
+                            StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
+                        )
+                        .with_message("Ability definition of module mismatch".to_string()));
+                    }
+                }
+            }
+
             for struct_def in module.struct_defs() {
                 let idx = struct_refs[struct_def.struct_handle.0 as usize];
                 let field_count = cache.structs[idx.0].fields.len() as u16;
@@ -2171,6 +2166,29 @@ impl Script {
             );
         }
 
+        for struct_handle in script.struct_handles() {
+            let struct_name = script.identifier_at(struct_handle.name);
+            let module_handle = script.module_handle_at(struct_handle.module);
+            let module_id = script.module_id_for_handle(module_handle);
+            let (_, struct_) = cache
+                .resolve_struct_by_name(struct_name, &module_id)
+                .map_err(|err| err.finish(Location::Script))?;
+            if !struct_handle.abilities.is_subset(struct_.abilities)
+                || &struct_handle
+                    .type_parameters
+                    .iter()
+                    .map(|ty| ty.is_phantom)
+                    .collect::<Vec<_>>()
+                    != &struct_.phantom_ty_args_mask
+            {
+                return Err(
+                    PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
+                        .with_message("Ability definition of module mismatch".to_string())
+                        .finish(Location::Script),
+                );
+            }
+        }
+
         let mut function_refs = vec![];
         for func_handle in script.function_handles().iter() {
             let func_name = script.identifier_at(func_handle.name);

third_party/move/move-vm/runtime/src/loader/type_loader.rs

@@ -4,20 +4,16 @@
 use move_binary_format::{
     binary_views::BinaryIndexedView, errors::PartialVMResult, file_format::SignatureToken,
 };
-use move_core_types::{identifier::IdentStr, language_storage::ModuleId};
-use move_vm_types::loaded_data::runtime_types::{CachedStructIndex, StructType, Type};
+use move_core_types::language_storage::ModuleId;
+use move_vm_types::loaded_data::runtime_types::{StructName, Type};
 use std::sync::Arc;
 
 // `make_type_internal` returns a `Type` given a signature and a resolver which
 // is resonsible to map a local struct index to a global one
-pub fn make_type_internal<F>(
+pub fn make_type_internal(
     module: BinaryIndexedView,
     tok: &SignatureToken,
-    resolver: &F,
-) -> PartialVMResult<Type>
-where
-    F: Fn(&IdentStr, &ModuleId) -> PartialVMResult<(CachedStructIndex, Arc<StructType>)>,
-{
+) -> PartialVMResult<Type> {
     let res = match tok {
         SignatureToken::Bool => Type::Bool,
         SignatureToken::U8 => Type::U8,
@@ -30,15 +26,15 @@ where
         SignatureToken::Signer => Type::Signer,
         SignatureToken::TypeParameter(idx) => Type::TyParam(*idx),
         SignatureToken::Vector(inner_tok) => {
-            let inner_type = make_type_internal(module, inner_tok, resolver)?;
+            let inner_type = make_type_internal(module, inner_tok)?;
             Type::Vector(Box::new(inner_type))
         },
         SignatureToken::Reference(inner_tok) => {
-            let inner_type = make_type_internal(module, inner_tok, resolver)?;
+            let inner_type = make_type_internal(module, inner_tok)?;
             Type::Reference(Box::new(inner_type))
         },
         SignatureToken::MutableReference(inner_tok) => {
-            let inner_type = make_type_internal(module, inner_tok, resolver)?;
+            let inner_type = make_type_internal(module, inner_tok)?;
             Type::MutableReference(Box::new(inner_type))
         },
         SignatureToken::Struct(sh_idx) => {
@@ -49,16 +45,18 @@ where
                 *module.address_identifier_at(module_handle.address),
                 module.identifier_at(module_handle.name).to_owned(),
             );
-            let (_, struct_) = resolver(struct_name, &module_id)?;
             Type::Struct {
-                name: struct_.name.clone(),
-                ability: struct_.abilities,
+                name: Arc::new(StructName {
+                    name: struct_name.to_owned(),
+                    module: module_id,
+                }),
+                ability: struct_handle.abilities,
             }
         },
         SignatureToken::StructInstantiation(sh_idx, tys) => {
             let type_parameters: Vec<_> = tys
                 .iter()
-                .map(|tok| make_type_internal(module, tok, resolver))
+                .map(|tok| make_type_internal(module, tok))
                 .collect::<PartialVMResult<_>>()?;
             let struct_handle = module.struct_handle_at(*sh_idx);
             let struct_name = module.identifier_at(struct_handle.name);
@@ -67,12 +65,14 @@ where
                 *module.address_identifier_at(module_handle.address),
                 module.identifier_at(module_handle.name).to_owned(),
             );
-            let (_, struct_) = resolver(struct_name, &module_id)?;
             Type::StructInstantiation {
-                name: struct_.name.clone(),
-                base_ability_set: struct_.abilities,
+                name: Arc::new(StructName {
+                    name: struct_name.to_owned(),
+                    module: module_id,
+                }),
+                base_ability_set: struct_handle.abilities,
                 ty_args: type_parameters,
-                phantom_ty_args_mask: struct_
+                phantom_ty_args_mask: struct_handle
                     .type_parameters
                     .iter()
                     .map(|ty| ty.is_phantom)

third_party/move/move-vm/types/Cargo.toml

@@ -10,6 +10,7 @@ publish = false
 edition = "2021"
 
 [dependencies]
+derivative = "2.2.0"
 once_cell = "1.7.2"
 proptest = { version = "1.0.0", optional = true }
 serde = { version = "1.0.124", features = ["derive", "rc"] }

third_party/move/move-vm/types/src/loaded_data/runtime_types.rs

@@ -2,6 +2,7 @@
 // Copyright (c) The Move Contributors
 // SPDX-License-Identifier: Apache-2.0
 
+use derivative::Derivative;
 use move_binary_format::{
     errors::{PartialVMError, PartialVMResult},
     file_format::{
@@ -135,7 +136,16 @@ pub struct StructName {
     pub name: Identifier,
 }
 
-#[derive(Debug, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Derivative)]
+#[derivative(
+    Debug,
+    Clone,
+    Eq,
+    Hash,
+    Ord = "feature_allow_slow_enum",
+    PartialEq,
+    PartialOrd = "feature_allow_slow_enum"
+)]
 pub enum Type {
     Bool,
     U8,
@@ -146,12 +156,30 @@ pub enum Type {
     Vector(Box<Type>),
     Struct {
         name: Arc<StructName>,
+        #[derivative(
+            PartialEq = "ignore",
+            Hash = "ignore",
+            PartialOrd = "ignore",
+            Ord = "ignore"
+        )]
         ability: AbilitySet,
     },
     StructInstantiation {
         name: Arc<StructName>,
         ty_args: Vec<Type>,
+        #[derivative(
+            PartialEq = "ignore",
+            Hash = "ignore",
+            PartialOrd = "ignore",
+            Ord = "ignore"
+        )]
         base_ability_set: AbilitySet,
+        #[derivative(
+            PartialEq = "ignore",
+            Hash = "ignore",
+            PartialOrd = "ignore",
+            Ord = "ignore"
+        )]
         phantom_ty_args_mask: Vec<bool>,
     },
     Reference(Box<Type>),
@@ -193,10 +221,7 @@ impl Type {
             Type::MutableReference(ty) => {
                 Type::MutableReference(Box::new(ty.apply_subst(subst, depth + 1)?))
             },
-            Type::Struct {
-                name,
-                ability,
-            } => Type::Struct {
+            Type::Struct { name, ability } => Type::Struct {
                 name: name.clone(),
                 ability: *ability,
             },