move: leak verifier returns execution error (MystenLabs#7495)

rvantonder · web-flow · commit 27bee572aa62 · 2023-01-24T11:00:54.000-06:00
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/direct_leak_through_call.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/direct_leak_through_call.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-25:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID leaked through function call."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID leaked through function call.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/indirect_leak_through_call.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/indirect_leak_through_call.exp
@@ -1,9 +1,9 @@
 processed 2 tasks
 
 task 0 'publish'. lines 4-25:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a struct."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a struct.") } }
 
 task 1 'publish'. lines 27-48:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID leaked through function call."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID leaked through function call.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/through_call_with_borrow_field.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/through_call_with_borrow_field.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-34:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID leaked through function call."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID leaked through function call.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/through_direct_return.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/through_direct_return.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-19:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID leaked through function return."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID leaked through function return.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/through_indirect_return.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/through_indirect_return.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-19:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a struct."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a struct.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/through_pack.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/through_pack.exp
@@ -1,9 +1,9 @@
 processed 2 tasks
 
 task 0 'publish'. lines 4-36:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a struct."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("test") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a struct.") } }
 
 task 1 'publish'. lines 38-60:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a struct."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a struct.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/through_vector.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/through_vector.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-20:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a vector"), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a vector.") } }
diff --git a/crates/sui-verifier-transactional-tests/tests/id_leak/transmute.exp b/crates/sui-verifier-transactional-tests/tests/id_leak/transmute.exp
@@ -1,5 +1,5 @@
 processed 1 task
 
 task 0 'publish'. lines 4-28:
-Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.
-Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: VMVerificationOrDeserializationError, source: Some(VMError { major_status: UNKNOWN_VERIFICATION_ERROR, sub_status: None, message: Some("Sui Move Bytecode Verification Error: ID is leaked into a struct."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("ID is leaked into a struct.") } }
diff --git a/crates/sui-verifier/src/id_leak_verifier.rs b/crates/sui-verifier/src/id_leak_verifier.rs
@@ -14,7 +14,7 @@
 //! 4. Passed to a function cal::;
 use move_binary_format::{
     binary_views::{BinaryIndexedView, FunctionView},
-    errors::{Location, PartialVMError, PartialVMResult},
+    errors::PartialVMError,
     file_format::{
         Bytecode, CodeOffset, CompiledModule, FunctionDefinitionIndex, FunctionHandle, LocalIndex,
         StructDefinition, StructFieldInformation,
@@ -23,9 +23,13 @@ use move_binary_format::{
 use move_bytecode_verifier::absint::{
     AbstractDomain, AbstractInterpreter, JoinResult, TransferFunctions,
 };
-use move_core_types::vm_status::StatusCode;
 use std::collections::BTreeMap;
-use sui_types::{error::ExecutionError, id::OBJECT_MODULE_NAME, SUI_FRAMEWORK_ADDRESS};
+use sui_types::{
+    error::ExecutionError, id::OBJECT_MODULE_NAME, messages::ExecutionFailureStatus,
+    SUI_FRAMEWORK_ADDRESS,
+};
+
+use crate::verification_failure;
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 enum AbstractValue {
@@ -59,9 +63,7 @@ fn verify_id_leak(module: &CompiledModule) -> Result<(), ExecutionError> {
             FunctionView::function(module, FunctionDefinitionIndex(index as u16), code, handle);
         let initial_state = AbstractState::new(&func_view);
         let mut verifier = IDLeakAnalysis::new(&binary_view, &func_view);
-        verifier
-            .analyze_function(initial_state, &func_view)
-            .map_err(|e| e.finish(Location::Module(module.self_id())))?;
+        verifier.analyze_function(initial_state, &func_view)?;
     }
 
     Ok(())
@@ -123,7 +125,7 @@ impl<'a> IDLeakAnalysis<'a> {
 }
 
 impl<'a> TransferFunctions for IDLeakAnalysis<'a> {
-    type Error = PartialVMError;
+    type Error = ExecutionError;
     type State = AbstractState;
 
     fn execute(
@@ -132,7 +134,7 @@ impl<'a> TransferFunctions for IDLeakAnalysis<'a> {
         bytecode: &Bytecode,
         index: CodeOffset,
         last_index: CodeOffset,
-    ) -> PartialVMResult<()> {
+    ) -> Result<(), ExecutionError> {
         execute_inner(self, state, bytecode, index)?;
         // invariant: the stack should be empty at the end of the block
         // If it is not, something is wrong with the implementation, so throw an invariant
@@ -142,8 +144,8 @@ impl<'a> TransferFunctions for IDLeakAnalysis<'a> {
                 false,
                 "Invalid stack transitions. Non-zero stack size at the end of the block",
             );
-            return Err(PartialVMError::new(
-                StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
+            return Err(ExecutionError::from_kind(
+                ExecutionFailureStatus::InvariantViolation,
             ));
         }
         Ok(())
@@ -179,14 +181,19 @@ fn is_call_safe_to_leak(verifier: &IDLeakAnalysis, function_handle: &FunctionHan
                 == "delete_child_object")
 }
 
-fn call(verifier: &mut IDLeakAnalysis, function_handle: &FunctionHandle) -> PartialVMResult<()> {
+fn call(
+    verifier: &mut IDLeakAnalysis,
+    function_handle: &FunctionHandle,
+) -> Result<(), ExecutionError> {
     let guaranteed_safe = is_call_safe_to_leak(verifier, function_handle);
     let parameters = verifier
         .binary_view
         .signature_at(function_handle.parameters);
     for _ in 0..parameters.len() {
         if verifier.stack.pop().unwrap() == AbstractValue::ID && !guaranteed_safe {
-            return Err(move_verification_error("ID leaked through function call."));
+            return Err(verification_failure(
+                "ID leaked through function call.".to_string(),
+            ));
         }
     }
 
@@ -204,11 +211,16 @@ fn num_fields(struct_def: &StructDefinition) -> usize {
     }
 }
 
-fn pack(verifier: &mut IDLeakAnalysis, struct_def: &StructDefinition) -> PartialVMResult<()> {
+fn pack(
+    verifier: &mut IDLeakAnalysis,
+    struct_def: &StructDefinition,
+) -> Result<(), ExecutionError> {
     for _ in 0..num_fields(struct_def) {
         let value = verifier.stack.pop().unwrap();
         if value == AbstractValue::ID {
-            return Err(move_verification_error("ID is leaked into a struct."));
+            return Err(verification_failure(
+                "ID is leaked into a struct.".to_string(),
+            ));
         }
     }
     verifier.stack.push(AbstractValue::NonID);
@@ -239,7 +251,7 @@ fn execute_inner(
     state: &mut AbstractState,
     bytecode: &Bytecode,
     _: CodeOffset,
-) -> PartialVMResult<()> {
+) -> Result<(), ExecutionError> {
     // TODO: Better diagnostics with location
     match bytecode {
         Bytecode::Pop => {
@@ -309,7 +321,7 @@ fn execute_inner(
             // Top of stack is the reference, and the second element is the value.
             verifier.stack.pop().unwrap();
             if verifier.stack.pop().unwrap() == AbstractValue::ID {
-                return Err(move_verification_error("ID is leaked to a reference."));
+                return Err(verification_failure("ID is leaked to a reference.".to_string()));
             }
         }
 
@@ -353,7 +365,7 @@ fn execute_inner(
         Bytecode::Ret => {
             for _ in 0..verifier.function_view.return_().len() {
                 if verifier.stack.pop().unwrap() == AbstractValue::ID {
-                    return Err(move_verification_error("ID leaked through function return."));
+		    return Err(verification_failure("ID leaked through function return.".to_string()));
                 }
             }
         }
@@ -389,15 +401,15 @@ fn execute_inner(
         Bytecode::VecPack(_, num) => {
             for _ in 0..*num {
                 if verifier.stack.pop().unwrap() == AbstractValue::ID {
-                    return Err(move_verification_error("ID is leaked into a vector"));
+		    return Err(verification_failure("ID is leaked into a vector.".to_string()));
                 }
             }
             verifier.stack.push(AbstractValue::NonID);
         }
 
         Bytecode::VecPushBack(_) => {
             if verifier.stack.pop().unwrap() == AbstractValue::ID {
-                return Err(move_verification_error("ID is leaked into a vector"));
+		return Err(verification_failure("ID is leaked into a vector.".to_string()));
             }
             verifier.stack.pop().unwrap();
         }
@@ -419,7 +431,7 @@ fn execute_inner(
     Ok(())
 }
 
-fn expect_ok<T>(res: Result<T, PartialVMError>) -> PartialVMResult<T> {
+fn expect_ok<T>(res: Result<T, PartialVMError>) -> Result<T, ExecutionError> {
     match res {
         Ok(x) => Ok(x),
         Err(partial_vm_error) => {
@@ -429,15 +441,9 @@ fn expect_ok<T>(res: Result<T, PartialVMError>) -> PartialVMResult<T> {
                 Got error: {partial_vm_error:?}"
             );
             // This is an internal error, but we cannot accept the module as safe
-            Err(PartialVMError::new(
-                StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
+            Err(ExecutionError::from_kind(
+                ExecutionFailureStatus::InvariantViolation,
             ))
         }
     }
 }
-
-#[must_use]
-fn move_verification_error(msg: impl std::fmt::Display) -> PartialVMError {
-    PartialVMError::new(StatusCode::UNKNOWN_VERIFICATION_ERROR)
-        .with_message(format!("Sui Move Bytecode Verification Error: {}", msg))
-}
