[verifier] limit the number of back edges

starcoinorg · Feb 28, 2023 · ef2f4d4 · ef2f4d4
1 parent b110827
commit ef2f4d4
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 5 deletions.
diff --git a/language/move-binary-format/src/control_flow_graph.rs b/language/move-binary-format/src/control_flow_graph.rs
@@ -45,6 +45,9 @@ pub trait ControlFlowGraph {
     /// Checks if the the edge from cur->next is a back edge
     /// returns false if the edge is not in the cfg
     fn is_back_edge(&self, cur: BlockId, next: BlockId) -> bool;
+
+    /// Return the number of back edges in the cfg
+    fn num_back_edges(&self) -> usize;
 }
 
 struct BasicBlock {
@@ -325,4 +328,10 @@ impl ControlFlowGraph for VMControlFlowGraph {
             .get(&next)
             .map_or(false, |back_edges| back_edges.contains(&cur))
     }
+
+    fn num_back_edges(&self) -> usize {
+        self.loop_heads
+            .iter()
+            .fold(0, |acc, (_, edges)| acc + edges.len())
+    }
 }
diff --git a/language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/signature_tests.rs b/language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/signature_tests.rs
@@ -228,6 +228,9 @@ fn big_signature_test() {
             max_struct_definitions: Some(200),
             max_fields_in_struct: Some(30),
             max_function_definitions: Some(1000),
+            max_back_edges_per_function: Some(20),
+            max_back_edges_per_module: Some(400),
+            max_basic_blocks_in_script: Some(1024),
         },
         &module,
     )

diff --git a/language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/vec_pack_tests.rs b/language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/vec_pack_tests.rs
@@ -72,6 +72,9 @@ fn test_vec_pack() {
             max_struct_definitions: Some(200),
             max_fields_in_struct: Some(30),
             max_function_definitions: Some(1000),
+            max_back_edges_per_function: Some(20),
+            max_back_edges_per_module: Some(400),
+            max_basic_blocks_in_script: Some(1024),
         },
         &m,
     )

diff --git a/language/move-bytecode-verifier/src/code_unit_verifier.rs b/language/move-bytecode-verifier/src/code_unit_verifier.rs
@@ -47,16 +47,23 @@ impl<'a> CodeUnitVerifier<'a> {
             let fh = module.function_handle_at(func_def.function);
             name_def_map.insert(fh.name, FunctionDefinitionIndex(idx as u16));
         }
+        let mut total_back_edges = 0;
         for (idx, function_definition) in module.function_defs().iter().enumerate() {
             let index = FunctionDefinitionIndex(idx as TableIndex);
-            Self::verify_function(
+            let num_back_edges = Self::verify_function(
                 verifier_config,
                 index,
                 function_definition,
                 module,
                 &name_def_map,
             )
-            .map_err(|err| err.at_index(IndexKind::FunctionDefinition, index.0))?
+            .map_err(|err| err.at_index(IndexKind::FunctionDefinition, index.0))?;
+            total_back_edges += num_back_edges;
+        }
+        if let Some(limit) = verifier_config.max_back_edges_per_module {
+            if total_back_edges > limit {
+                return Err(PartialVMError::new(StatusCode::TOO_MANY_BASIC_BLOCKS));
+            }
         }
         Ok(())
     }
@@ -76,6 +83,19 @@ impl<'a> CodeUnitVerifier<'a> {
         let function_view = control_flow::verify_script(verifier_config, script)?;
         let resolver = BinaryIndexedView::Script(script);
         let name_def_map = HashMap::new();
+
+        if let Some(limit) = verifier_config.max_basic_blocks_in_script {
+            if function_view.cfg().blocks().len() > limit {
+                return Err(PartialVMError::new(StatusCode::TOO_MANY_BASIC_BLOCKS));
+            }
+        }
+
+        if let Some(limit) = verifier_config.max_back_edges_per_function {
+            if function_view.cfg().num_back_edges() > limit {
+                return Err(PartialVMError::new(StatusCode::TOO_MANY_BACK_EDGES));
+            }
+        }
+
         //verify
         let code_unit_verifier = CodeUnitVerifier {
             resolver,
@@ -91,11 +111,11 @@ impl<'a> CodeUnitVerifier<'a> {
         function_definition: &FunctionDefinition,
         module: &CompiledModule,
         name_def_map: &HashMap<IdentifierIndex, FunctionDefinitionIndex>,
-    ) -> PartialVMResult<()> {
+    ) -> PartialVMResult<usize> {
         // nothing to verify for native function
         let code = match &function_definition.code {
             Some(code) => code,
-            None => return Ok(()),
+            None => return Ok(0),
         };
 
         // create `FunctionView` and `BinaryIndexedView`
@@ -115,6 +135,15 @@ impl<'a> CodeUnitVerifier<'a> {
             }
         }
 
+        let num_back_edges = function_view.cfg().num_back_edges();
+        if let Some(limit) = verifier_config.max_back_edges_per_function {
+            if num_back_edges > limit {
+                return Err(
+                    PartialVMError::new(StatusCode::TOO_MANY_BACK_EDGES).at_code_offset(index, 0)
+                );
+            }
+        }
+
         let resolver = BinaryIndexedView::Module(module);
         // verify
         let code_unit_verifier = CodeUnitVerifier {
@@ -123,7 +152,9 @@ impl<'a> CodeUnitVerifier<'a> {
             name_def_map,
         };
         code_unit_verifier.verify_common(verifier_config)?;
-        AcquiresVerifier::verify(module, index, function_definition)
+        AcquiresVerifier::verify(module, index, function_definition)?;
+
+        Ok(num_back_edges)
     }
 
     fn verify_common(&self, verifier_config: &VerifierConfig) -> PartialVMResult<()> {

diff --git a/language/move-bytecode-verifier/src/verifier.rs b/language/move-bytecode-verifier/src/verifier.rs
@@ -31,6 +31,9 @@ pub struct VerifierConfig {
     pub max_struct_definitions: Option<usize>,
     pub max_fields_in_struct: Option<usize>,
     pub max_function_definitions: Option<usize>,
+    pub max_back_edges_per_function: Option<usize>,
+    pub max_back_edges_per_module: Option<usize>,
+    pub max_basic_blocks_in_script: Option<usize>,
 }
 
 /// Helper for a "canonical" verification of a module.
@@ -145,6 +148,9 @@ impl Default for VerifierConfig {
             // max_struct_definitions: Some(200),
             // max_fields_in_struct: Some(30),
             // max_function_definitions: Some(1000),
+            max_back_edges_per_function: None,
+            max_back_edges_per_module: None,
+            max_basic_blocks_in_script: None,
         }
     }
 }
diff --git a/language/move-core/types/src/vm_status.rs b/language/move-core/types/src/vm_status.rs
@@ -641,6 +641,14 @@ pub enum StatusCode {
     MAX_STRUCT_DEFINITIONS_REACHED = 1120,
     MAX_FIELD_DEFINITIONS_REACHED = 1121,
 
+    // Reserved error code for future use
+    TOO_MANY_BACK_EDGES = 1122,
+    RESERVED_VERIFICATION_ERROR_1 = 1123,
+    RESERVED_VERIFICATION_ERROR_2 = 1124,
+    RESERVED_VERIFICATION_ERROR_3 = 1125,
+    RESERVED_VERIFICATION_ERROR_4 = 1126,
+    RESERVED_VERIFICATION_ERROR_5 = 1127,
+
     // These are errors that the VM might raise if a violation of internal
     // invariants takes place.
     // Invariant Violation Errors: 2000-2999