[verifier] Added type-checking of templated type arguments for entry …

…functions (MystenLabs#469)

* [verifier] Added type-checking of templated type arguments for entry functions

* Addressed review comments

* Addressed additional review comments

* Removed unnecesary checks

sui_programmability/adapter/src/adapter.rs

@@ -800,11 +800,10 @@ fn is_param_tx_context(param: &Type) -> bool {
                 address,
                 module,
                 name,
-                type_arguments,
+                ..
             } if address == &SUI_FRAMEWORK_ADDRESS
                 && module.as_ident_str() == TX_CONTEXT_MODULE_NAME
-                && name.as_ident_str() == TX_CONTEXT_STRUCT_NAME
-                && type_arguments.is_empty() =>
+                && name.as_ident_str() == TX_CONTEXT_STRUCT_NAME =>
             {
                 return true
             }

sui_programmability/adapter/src/unit_tests/data/simple_call/sources/M1.move

@@ -3,12 +3,17 @@ module Test::M1 {
     use FastX::ID::ID;
     use FastX::TxContext::{Self, TxContext};
     use FastX::Transfer;
+    use FastX::Coin::Coin;
 
     struct Object has key, store {
         id: ID,
         value: u64,
     }
 
+    fun foo<T: key, T2: drop>(_p1: u64, value1: T, _value2: &Coin<T2>, _p2: u64): T {
+        value1 
+    }
+
     public fun create(value: u64, recipient: vector<u8>, ctx: &mut TxContext) {
         Transfer::transfer(
             Object { id: TxContext::new_id(ctx), value },

sui_programmability/verifier/Cargo.toml

@@ -3,13 +3,15 @@ name = "sui-verifier"
 version = "0.1.0"
 edition = "2021"
 authors = ["Mysten Labs <[email protected]>"]
-description = "Move framework for fastX platform"
+description = "Move framework for Sui platform"
 license = "Apache-2.0"
 publish = false
 
 [dependencies]
 move-binary-format = { git = "https://github.com/diem/move", rev = "7683d09732dd930c581583bf5fde97fb7ac02ff7" }
 move-bytecode-verifier = { git = "https://github.com/diem/move", rev = "7683d09732dd930c581583bf5fde97fb7ac02ff7" }
 move-core-types = { git = "https://github.com/diem/move", rev = "7683d09732dd930c581583bf5fde97fb7ac02ff7", features = ["address20"] }
+move-disassembler = { git = "https://github.com/diem/move", rev = "7683d09732dd930c581583bf5fde97fb7ac02ff7" }
+move-ir-types = { git = "https://github.com/diem/move", rev = "7683d09732dd930c581583bf5fde97fb7ac02ff7" }
 
 sui-types = { path = "../../sui_types" }

sui_programmability/verifier/src/lib.rs

@@ -6,6 +6,7 @@ pub mod verifier;
 pub mod global_storage_access_verifier;
 pub mod id_immutable_verifier;
 pub mod id_leak_verifier;
+pub mod param_typecheck_verifier;
 pub mod struct_with_key_verifier;
 
 use sui_types::error::SuiError;

sui_programmability/verifier/src/param_typecheck_verifier.rs

@@ -0,0 +1,234 @@
+// Copyright (c) Mysten Labs
+// SPDX-License-Identifier: Apache-2.0
+
+use move_binary_format::{
+    binary_views::BinaryIndexedView,
+    file_format::{Ability, FunctionHandle, Signature, SignatureToken, Visibility},
+    CompiledModule,
+};
+use sui_types::{
+    base_types::{TX_CONTEXT_MODULE_NAME, TX_CONTEXT_STRUCT_NAME},
+    error::{SuiError, SuiResult},
+    SUI_FRAMEWORK_ADDRESS,
+};
+
+pub fn verify_module(module: &CompiledModule) -> SuiResult {
+    check_params(module)
+}
+/// Checks if parameters of functions that can become entry
+/// functions (functions called directly from Sui) have correct types.
+///
+/// We first identify functions that can be entry functions by looking
+/// for functions with the following properties:
+/// 1. Public
+/// 2. No return value
+/// 3. Parameter order: objects, primitives, &mut TxContext
+///
+/// Note that this can be ambiguous in presence of the following
+/// templated parameters:
+/// - param: T
+/// - param: vector<T> // and nested vectors
+///
+/// A function is considered an entry function if such templated
+/// arguments are part of "object parameters" only.
+///
+/// In order for the parameter types of an entry function to be
+/// correct, all generic types used in templated arguments mentioned
+/// above must have the `key` ability.
+pub fn check_params(module: &CompiledModule) -> SuiResult {
+    let view = BinaryIndexedView::Module(module);
+    for func_def in module.function_defs.iter() {
+        // find candidate entry functions and checke their parameters
+        // (ignore other functions)
+        if func_def.visibility != Visibility::Public {
+            // a non-public function cannot be called from Sui
+            continue;
+        }
+        let handle = view.function_handle_at(func_def.function);
+        if !view.signature_at(handle.return_).is_empty() {
+            // entry functions do not return values
+            continue;
+        }
+        let params = view.signature_at(handle.parameters);
+        let param_num = match is_entry_candidate(&view, params) {
+            Some(v) => v,
+            None => continue,
+        };
+        // iterate over all object params and make sure that each
+        // template-typed (either by itself or in a vector) argument
+        // has the Key ability
+        for (pos, p) in params.0[0..param_num].iter().enumerate() {
+            if let SignatureToken::TypeParameter(_) = p {
+                if !is_template_param_ok(handle, p) {
+                    return Err(SuiError::ModuleVerificationFailure {
+                        error: format!(
+                            "No `key` ability for the template parameter at position {} in entry function {}",
+                            pos,
+                            view.identifier_at(handle.name)
+                        ),
+                    });
+                }
+            }
+            if let SignatureToken::Vector(_) = p {
+                if !is_template_vector_param_ok(handle, p) {
+                    return Err(SuiError::ModuleVerificationFailure {
+                        error: format!(
+                            "No `key` ability for the template vector parameter at position {} in entry function {}",
+                            pos,
+                            view.identifier_at(handle.name)
+                        ),
+                    });
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
+/// Checks if a function can possibly be an entry function (without
+/// checking correctness of the function's parameters).
+fn is_entry_candidate(view: &BinaryIndexedView, params: &Signature) -> Option<usize> {
+    let mut obj_params_num = 0;
+    if params.is_empty() {
+        // must have at least on &mut TxContext param
+        return None;
+    }
+    let last_param = params.0.get(params.len() - 1).unwrap();
+    if !is_tx_context(view, last_param) {
+        return None;
+    }
+    if params.len() == 1 {
+        // only one &mut TxContext param
+        return Some(obj_params_num);
+    }
+    // currently, an entry function has object parameters followed by
+    // primitive type parameters (followed by the &mut TxContext
+    // param, but we already checked this one)
+    let mut primitive_params_phase = false; // becomes true once we start seeing primitive type params
+    for p in &params.0[0..params.len() - 1] {
+        if is_primitive_type(p) {
+            primitive_params_phase = true;
+        } else {
+            obj_params_num += 1;
+            if primitive_params_phase {
+                // We encounter a non primitive type parameter after
+                // the first one was encountered. This cannot be an
+                // entry function as it would get rejected by the
+                // resolve_and_type_check function in the adapter upon
+                // its call attempt from Sui
+                return None;
+            }
+            if !is_object(view, p)
+                && !is_template(p)
+                && !is_object_vector(view, p)
+                && !is_template_vector(p)
+            {
+                // A non-primitive type for entry functions must be an
+                // object, or a generic type, or a vector (possibly
+                // nested) of objects, or a templeted vector (possibly
+                // nested). Otherwise it is not an entry function as
+                // we cannot pass non-object types from Sui).
+                return None;
+            }
+        }
+    }
+    Some(obj_params_num)
+}
+
+/// Checks if a given parameter is of a primitive type. It's a mirror
+/// of the is_primitive function in the adapter module that operates
+/// on Type-s.
+fn is_primitive_type(p: &SignatureToken) -> bool {
+    use SignatureToken::*;
+    match p {
+        Bool | U8 | U64 | U128 | Address => true,
+        Vector(t) => is_primitive_type(t),
+        Signer
+        | Struct(_)
+        | StructInstantiation(..)
+        | Reference(_)
+        | MutableReference(_)
+        | TypeParameter(_) => false,
+    }
+}
+
+fn is_object(view: &BinaryIndexedView, p: &SignatureToken) -> bool {
+    use SignatureToken::*;
+    match p {
+        Struct(idx) => view
+            .struct_handle_at(*idx)
+            .abilities
+            .has_ability(Ability::Key),
+        StructInstantiation(idx, _) => view
+            .struct_handle_at(*idx)
+            .abilities
+            .has_ability(Ability::Key),
+        Reference(t) => is_object(view, t),
+        MutableReference(t) => is_object(view, t),
+        _ => false,
+    }
+}
+
+fn is_template(p: &SignatureToken) -> bool {
+    matches!(p, SignatureToken::TypeParameter(_))
+}
+
+fn is_object_vector(view: &BinaryIndexedView, p: &SignatureToken) -> bool {
+    if let SignatureToken::Vector(t) = p {
+        match &**t {
+            SignatureToken::Vector(inner_t) => return is_object_vector(view, inner_t),
+            other => return is_object(view, other),
+        }
+    }
+    false
+}
+
+fn is_template_vector(p: &SignatureToken) -> bool {
+    match p {
+        SignatureToken::Vector(t) => is_template_vector(t),
+        other => matches!(other, SignatureToken::TypeParameter(_)),
+    }
+}
+
+fn is_template_param_ok(handle: &FunctionHandle, p: &SignatureToken) -> bool {
+    if let SignatureToken::TypeParameter(idx) = p {
+        if !handle
+            .type_parameters
+            .get(*idx as usize)
+            .unwrap()
+            .has_ability(Ability::Key)
+        {
+            return false;
+        }
+    }
+    true
+}
+
+fn is_template_vector_param_ok(handle: &FunctionHandle, p: &SignatureToken) -> bool {
+    match p {
+        SignatureToken::Vector(t) => is_template_vector_param_ok(handle, t),
+        SignatureToken::TypeParameter(_) => is_template_param_ok(handle, p),
+        _ => true,
+    }
+}
+
+/// It's a mirror of the is_param_tx_context function in the adapter
+/// module that operates on Type-s.
+fn is_tx_context(view: &BinaryIndexedView, p: &SignatureToken) -> bool {
+    match p {
+        SignatureToken::MutableReference(m) => match &**m {
+            SignatureToken::Struct(idx) => {
+                let struct_handle = view.struct_handle_at(*idx);
+                let struct_name = view.identifier_at(struct_handle.name);
+                let module = view.module_handle_at(struct_handle.module);
+                let module_name = view.identifier_at(module.name);
+                let module_addr = view.address_identifier_at(module.address);
+                module_name == TX_CONTEXT_MODULE_NAME
+                    && module_addr == &SUI_FRAMEWORK_ADDRESS
+                    && struct_name == TX_CONTEXT_STRUCT_NAME
+            }
+            _ => false,
+        },
+        _ => false,
+    }
+}

sui_programmability/verifier/src/verifier.rs

@@ -8,13 +8,14 @@ use sui_types::error::SuiResult;
 
 use crate::{
     global_storage_access_verifier, id_immutable_verifier, id_leak_verifier,
-    struct_with_key_verifier,
+    param_typecheck_verifier, struct_with_key_verifier,
 };
 
 /// Helper for a "canonical" verification of a module.
 pub fn verify_module(module: &CompiledModule) -> SuiResult {
     struct_with_key_verifier::verify_module(module)?;
     global_storage_access_verifier::verify_module(module)?;
     id_immutable_verifier::verify_module(module)?;
-    id_leak_verifier::verify_module(module)
+    id_leak_verifier::verify_module(module)?;
+    param_typecheck_verifier::verify_module(module)
 }