[bug fix][verifier] Fix bug in ID leak verifier (MystenLabs#6754)

* [bug fix][verifier] Fix bug in ID leak verifier

- The ID leak verifier was more permissive than desired, leading to potential re-uses of object IDs

crates/sui-verifier-transactional-tests/tests/id_leak/indirect_leak_through_call.exp

@@ -2,7 +2,7 @@ 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 leaked through function call."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+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: [] }) } }
 
 task 1 'publish'. lines 27-48:
 Error: Transaction Effects Status: Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.

crates/sui-verifier-transactional-tests/tests/id_leak/through_indirect_return.exp

@@ -2,4 +2,4 @@ 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: [] }) } }
+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: [] }) } }

crates/sui-verifier-transactional-tests/tests/id_leak/through_pack.exp

@@ -0,0 +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: [] }) } }
+
+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: [] }) } }

crates/sui-verifier-transactional-tests/tests/id_leak/through_pack.mvir

@@ -0,0 +1,60 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# publish
+module 0x0.test {
+    import 0x2.object;
+    import 0x2.transfer;
+    import 0x2.tx_context;
+
+    struct A has key {
+        id: object.UID
+    }
+
+    struct C has key {
+        id: object.UID
+    }
+
+    struct B {
+        id: object.UID
+    }
+
+    public entry test(x: Self.A) {
+        let id: object.UID;
+        let b: Self.B;
+        let c: Self.C;
+
+        label l0:
+        A { id } = move(x);
+        b = B { id: move(id) };
+        B { id } = move(b);
+        c = C { id: move(id) };
+
+        transfer.transfer<Self.C>(move(c), 0x1);
+        return;
+    }
+}
+
+//# publish
+module 0x0.m {
+    import 0x2.object;
+
+    struct Foo has key {
+        id: object.UID,
+    }
+
+    struct Bar {
+        v: u64,
+        id: object.UID,
+    }
+
+    foo(f: Self.Foo) {
+        let id: object.UID;
+        let b: Self.Bar;
+        label l0:
+        Foo { id } = move(f);
+        b = Bar { v: 0, id: move(id) };
+        abort 0;
+    }
+
+}

crates/sui-verifier-transactional-tests/tests/id_leak/transmute.exp

@@ -2,4 +2,4 @@ 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 leaked through function call."), exec_state: None, location: Module(ModuleId { address: _, name: Identifier("m") }), indices: [], offsets: [] }) } }
+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: [] }) } }

crates/sui-verifier/src/id_leak_verifier.rs

@@ -190,16 +190,15 @@ fn num_fields(struct_def: &StructDefinition) -> usize {
     }
 }
 
-fn pack(verifier: &mut IDLeakAnalysis, struct_def: &StructDefinition) {
-    let mut has_id = false;
+fn pack(verifier: &mut IDLeakAnalysis, struct_def: &StructDefinition) -> PartialVMResult<()> {
     for _ in 0..num_fields(struct_def) {
-        has_id |= verifier.stack.pop().unwrap() == AbstractValue::ID;
+        let value = verifier.stack.pop().unwrap();
+        if value == AbstractValue::ID {
+            return Err(move_verification_error("ID is leaked into a struct."));
+        }
     }
-    verifier.stack.push(if has_id {
-        AbstractValue::ID
-    } else {
-        AbstractValue::NonID
-    });
+    verifier.stack.push(AbstractValue::NonID);
+    Ok(())
 }
 
 fn unpack(verifier: &mut IDLeakAnalysis, struct_def: &StructDefinition) {
@@ -354,12 +353,12 @@ fn execute_inner(
 
         Bytecode::Pack(idx) => {
             let struct_def = expect_ok(verifier.binary_view.struct_def_at(*idx))?;
-            pack(verifier, struct_def);
+            pack(verifier, struct_def)?;
         }
         Bytecode::PackGeneric(idx) => {
             let struct_inst = expect_ok(verifier.binary_view.struct_instantiation_at(*idx))?;
             let struct_def = expect_ok(verifier.binary_view.struct_def_at(struct_inst.def))?;
-            pack(verifier, struct_def);
+            pack(verifier, struct_def)?;
         }
         Bytecode::Unpack(idx) => {
             let struct_def = expect_ok(verifier.binary_view.struct_def_at(*idx))?;