fix move instruction integrity

steelgeek091 · Jan 3, 2025 · 2c313b6 · 2c313b6
1 parent cdb0820
commit 2c313b6
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 16 deletions.
diff --git a/moveos/moveos-verifier/src/check_complexity.rs b/moveos/moveos-verifier/src/check_complexity.rs
@@ -3,7 +3,7 @@
 
 use move_binary_format::binary_views::BinaryIndexedView;
 use move_binary_format::errors::{PartialVMError, PartialVMResult};
-use move_binary_format::file_format::{Bytecode, CompiledScript, StructFieldInformation};
+use move_binary_format::file_format::{Bytecode, CompiledScript, StructFieldInformation, StructVariantInstantiationIndex, VariantFieldInstantiationIndex};
 use move_binary_format::file_format::{
     CodeUnit, FieldInstantiationIndex, FunctionInstantiationIndex, IdentifierIndex,
     ModuleHandleIndex, SignatureIndex, SignatureToken, StructDefInstantiationIndex, TableIndex,
@@ -191,6 +191,39 @@ impl<'a> BinaryComplexityMeter<'a> {
         Ok(())
     }
 
+    fn meter_struct_variant_instantiation(
+        &self,
+        struct_inst_idx: StructVariantInstantiationIndex,
+    ) -> PartialVMResult<()> {
+        let struct_variant_insts =
+            self.resolver
+                .struct_variant_instantiations()
+                .ok_or_else(|| {
+                    PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message(
+                        "Can't get enum type instantiation -- not a module.".to_string(),
+                    )
+                })?;
+        let struct_variant_inst = safe_get_table(struct_variant_insts, struct_inst_idx.0)?;
+        self.meter_signature(struct_variant_inst.type_parameters)
+    }
+
+    fn meter_variant_field_instantiation(
+        &self,
+        variant_field_inst_idx: VariantFieldInstantiationIndex,
+    ) -> PartialVMResult<()> {
+        let variant_field_insts =
+            self.resolver
+                .variant_field_instantiations()
+                .ok_or_else(|| {
+                    PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message(
+                        "Can't get variant field instantiations -- not a module.".to_string(),
+                    )
+                })?;
+        let field_inst = safe_get_table(variant_field_insts, variant_field_inst_idx.0)?;
+
+        self.meter_signature(field_inst.type_parameters)
+    }
+
     fn meter_function_defs(&self) -> PartialVMResult<()> {
         let func_defs = self.resolver.function_defs().ok_or_else(|| {
             PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
@@ -214,20 +247,26 @@ impl<'a> BinaryComplexityMeter<'a> {
             match instr {
                 CallGeneric(idx) => {
                     self.meter_function_instantiation(*idx)?;
-                }
+                },
                 PackGeneric(idx) | UnpackGeneric(idx) => {
                     self.meter_struct_instantiation(*idx)?;
-                }
+                },
+                PackVariantGeneric(idx) | UnpackVariantGeneric(idx) | TestVariantGeneric(idx) => {
+                    self.meter_struct_variant_instantiation(*idx)?;
+                },
                 ExistsGeneric(idx)
                 | MoveFromGeneric(idx)
                 | MoveToGeneric(idx)
                 | ImmBorrowGlobalGeneric(idx)
                 | MutBorrowGlobalGeneric(idx) => {
                     self.meter_struct_instantiation(*idx)?;
-                }
+                },
                 ImmBorrowFieldGeneric(idx) | MutBorrowFieldGeneric(idx) => {
                     self.meter_field_instantiation(*idx)?;
-                }
+                },
+                ImmBorrowVariantFieldGeneric(idx) | MutBorrowVariantFieldGeneric(idx) => {
+                    self.meter_variant_field_instantiation(*idx)?;
+                },
                 VecPack(idx, _)
                 | VecLen(idx)
                 | VecImmBorrow(idx)
@@ -237,18 +276,74 @@ impl<'a> BinaryComplexityMeter<'a> {
                 | VecUnpack(idx, _)
                 | VecSwap(idx) => {
                     self.meter_signature(*idx)?;
-                }
+                },
 
                 // List out the other options explicitly so there's a compile error if a new
                 // bytecode gets added.
-                Pop | Ret | Branch(_) | BrTrue(_) | BrFalse(_) | LdU8(_) | LdU16(_) | LdU32(_)
-                | LdU64(_) | LdU128(_) | LdU256(_) | LdConst(_) | CastU8 | CastU16 | CastU32
-                | CastU64 | CastU128 | CastU256 | LdTrue | LdFalse | Call(_) | Pack(_)
-                | Unpack(_) | ReadRef | WriteRef | FreezeRef | Add | Sub | Mul | Mod | Div
-                | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le
-                | Ge | CopyLoc(_) | MoveLoc(_) | StLoc(_) | MutBorrowLoc(_) | ImmBorrowLoc(_)
-                | MutBorrowField(_) | ImmBorrowField(_) | MutBorrowGlobal(_)
-                | ImmBorrowGlobal(_) | Exists(_) | MoveTo(_) | MoveFrom(_) | Abort | Nop => (),
+                Pop
+                | Ret
+                | Branch(_)
+                | BrTrue(_)
+                | BrFalse(_)
+                | LdU8(_)
+                | LdU16(_)
+                | LdU32(_)
+                | LdU64(_)
+                | LdU128(_)
+                | LdU256(_)
+                | LdConst(_)
+                | CastU8
+                | CastU16
+                | CastU32
+                | CastU64
+                | CastU128
+                | CastU256
+                | LdTrue
+                | LdFalse
+                | Call(_)
+                | Pack(_)
+                | Unpack(_)
+                | PackVariant(_)
+                | UnpackVariant(_)
+                | TestVariant(_)
+                | ReadRef
+                | WriteRef
+                | FreezeRef
+                | Add
+                | Sub
+                | Mul
+                | Mod
+                | Div
+                | BitOr
+                | BitAnd
+                | Xor
+                | Shl
+                | Shr
+                | Or
+                | And
+                | Not
+                | Eq
+                | Neq
+                | Lt
+                | Gt
+                | Le
+                | Ge
+                | CopyLoc(_)
+                | MoveLoc(_)
+                | StLoc(_)
+                | MutBorrowLoc(_)
+                | ImmBorrowLoc(_)
+                | MutBorrowField(_)
+                | ImmBorrowField(_)
+                | MutBorrowVariantField(_)
+                | ImmBorrowVariantField(_)
+                | MutBorrowGlobal(_)
+                | ImmBorrowGlobal(_)
+                | Exists(_)
+                | MoveTo(_)
+                | MoveFrom(_)
+                | Abort
+                | Nop => (),
             }
         }
         Ok(())
@@ -265,9 +360,17 @@ impl<'a> BinaryComplexityMeter<'a> {
                 StructFieldInformation::Native => continue,
                 StructFieldInformation::Declared(fields) => {
                     for field in fields {
-                        self.charge(field.signature.0.preorder_traversal().count() as u64)?;
+                        self.charge(field.signature.0.num_nodes() as u64)?;
                     }
-                }
+                },
+                StructFieldInformation::DeclaredVariants(variants) => {
+                    for variant in variants {
+                        self.meter_identifier(variant.name)?;
+                        for field in &variant.fields {
+                            self.charge(field.signature.0.num_nodes() as u64)?;
+                        }
+                    }
+                },
             }
         }
         Ok(())

diff --git a/moveos/moveos-verifier/src/metadata.rs b/moveos/moveos-verifier/src/metadata.rs
@@ -697,6 +697,16 @@ impl<'a> ExtendedChecker<'a> {
                     | Bytecode::VecPushBack(_)
                     | Bytecode::VecPopBack(_)
                     | Bytecode::VecUnpack(_, _)
+                    | Bytecode::PackVariant(_)
+                    | Bytecode::PackVariantGeneric(_)
+                    | Bytecode::UnpackVariant(_)
+                    | Bytecode::UnpackVariantGeneric(_)
+                    | Bytecode::TestVariant(_)
+                    | Bytecode::TestVariantGeneric(_)
+                    | Bytecode::MutBorrowVariantField(_)
+                    | Bytecode::MutBorrowVariantFieldGeneric(_)
+                    | Bytecode::ImmBorrowVariantField(_)
+                    | Bytecode::ImmBorrowVariantFieldGeneric(_)
                     | Bytecode::VecSwap(_) => {}
                 }
             }

diff --git a/moveos/moveos-verifier/src/verifier.rs b/moveos/moveos-verifier/src/verifier.rs
@@ -1809,6 +1809,16 @@ pub fn verify_global_storage_access(module: &CompiledModule) -> VMResult<bool> {
                     | Bytecode::VecPushBack(_)
                     | Bytecode::VecPopBack(_)
                     | Bytecode::VecUnpack(_, _)
+                    | Bytecode::PackVariant(_)
+                    | Bytecode::PackVariantGeneric(_)
+                    | Bytecode::UnpackVariant(_)
+                    | Bytecode::UnpackVariantGeneric(_)
+                    | Bytecode::TestVariant(_)
+                    | Bytecode::TestVariantGeneric(_)
+                    | Bytecode::MutBorrowVariantField(_)
+                    | Bytecode::MutBorrowVariantFieldGeneric(_)
+                    | Bytecode::ImmBorrowVariantField(_)
+                    | Bytecode::ImmBorrowVariantFieldGeneric(_)
                     | Bytecode::VecSwap(_) => {}
                 }
             }