[security] Cherry-picking recent security fixes from Aptos

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())
+    }
 }

language/move-borrow-graph/src/graph.rs

@@ -26,6 +26,14 @@ impl<Loc: Copy, Lbl: Clone + Ord> BorrowGraph<Loc, Lbl> {
         Self(BTreeMap::new())
     }
 
+    /// Returns the graph size, that is the number of nodes + number of edges
+    pub fn graph_size(&self) -> usize {
+        self.0
+            .values()
+            .map(|r| 1 + r.borrowed_by.0.values().map(|e| e.len()).sum::<usize>())
+            .sum()
+    }
+
     /// checks if the given reference is mutable or not
     pub fn is_mutable(&self, id: RefID) -> bool {
         self.0.get(&id).unwrap().mutable

language/move-bytecode-verifier/bytecode-verifier-tests/Cargo.toml

@@ -13,11 +13,12 @@ edition = "2021"
 petgraph = "0.5.1"
 proptest = "1.0.0"
 fail = { version = "0.4.0", features = ['failpoints']}
-
+hex = "0.4.3"
 move-bytecode-verifier = { path = "../" }
 invalid-mutations = { path = "../invalid-mutations" }
 move-core-types = { path = "../../move-core/types" }
-move-binary-format = { path = "../../move-binary-format", features = ["fuzzing"] }
+move-binary-format = { path = "../../move-binary-format", features = ["fuzzing" ] }
 
 [features]
 fuzzing = ["move-binary-format/fuzzing"]
+address32 = ["move-core-types/address32"]

language/move-bytecode-verifier/bytecode-verifier-tests/METER_TESTING.md

@@ -0,0 +1,5 @@
+This testsuite can be run in a specific way to print the time until a 'complex' program is detected or accepted. Call as in:
+
+```
+cargo test --release --features=address32   -- --nocapture 1>/dev/null
+```

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/catch_unwind.rs

@@ -9,6 +9,9 @@ use move_core_types::{
 };
 use std::panic::{self, PanicInfo};
 
+// TODO: this tests must run in its own process since otherwise any crashing test here
+//   secondary-crashes in the panic handler.
+#[ignore]
 #[test]
 fn test_unwind() {
     let scenario = FailScenario::setup();
@@ -19,7 +22,7 @@ fn test_unwind() {
     }));
 
     let m = empty_module();
-    let res = move_bytecode_verifier::verify_module_with_config(&VerifierConfig::default(), &m)
+    let res = move_bytecode_verifier::verify_module_with_config(&VerifierConfig::unbounded(), &m)
         .unwrap_err();
     assert_eq!(res.major_status(), StatusCode::VERIFIER_INVARIANT_VIOLATION);
     scenario.teardown();

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/code_unit_tests.rs

@@ -68,7 +68,7 @@ fn test_max_number_of_bytecode() {
     nops.push(Bytecode::Ret);
     let module = dummy_procedure_module(nops);
 
-    let result = CodeUnitVerifier::verify_module(&Default::default(), &module);
+    let result = CodeUnitVerifier::verify_module(&VerifierConfig::unbounded(), &module);
     assert!(result.is_ok());
 }
 

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/constants_tests.rs

@@ -14,6 +14,7 @@ proptest! {
 }
 
 #[test]
+#[cfg(not(feature = "address32"))]
 fn valid_primitives() {
     let mut module = empty_module();
     module.constant_pool = vec![
@@ -57,6 +58,7 @@ fn valid_primitives() {
 }
 
 #[test]
+#[cfg(not(feature = "address32"))]
 fn invalid_primitives() {
     malformed(SignatureToken::U8, vec![0, 0]);
     malformed(SignatureToken::U16, vec![0, 0, 0, 0]);
@@ -72,6 +74,7 @@ fn invalid_primitives() {
 }
 
 #[test]
+#[cfg(not(feature = "address32"))]
 fn valid_vectors() {
     let double_vec = |item: Vec<u8>| -> Vec<u8> {
         let mut items = vec![2];
@@ -193,6 +196,7 @@ fn valid_vectors() {
 }
 
 #[test]
+#[cfg(not(feature = "address32"))]
 fn invalid_vectors() {
     let double_vec = |item: Vec<u8>| -> Vec<u8> {
         let mut items = vec![2];
@@ -244,6 +248,7 @@ fn tvec(s: SignatureToken) -> SignatureToken {
     SignatureToken::Vector(Box::new(s))
 }
 
+#[allow(unused)]
 fn malformed(type_: SignatureToken, data: Vec<u8>) {
     error(type_, data, StatusCode::MALFORMED_CONSTANT_DATA)
 }

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/control_flow_tests.rs

@@ -8,7 +8,7 @@ use move_binary_format::{
     errors::PartialVMResult,
     file_format::{Bytecode, CompiledModule, FunctionDefinitionIndex, TableIndex},
 };
-use move_bytecode_verifier::{control_flow, VerifierConfig};
+use move_bytecode_verifier::{control_flow, meter::DummyMeter, VerifierConfig};
 use move_core_types::vm_status::StatusCode;
 
 fn verify_module(verifier_config: &VerifierConfig, module: &CompiledModule) -> PartialVMResult<()> {
@@ -30,6 +30,7 @@ fn verify_module(verifier_config: &VerifierConfig, module: &CompiledModule) -> P
             current_function,
             function_definition,
             code,
+            &mut DummyMeter,
         )?;
     }
     Ok(())

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/large_type_test.rs

@@ -0,0 +1,155 @@
+// Copyright (c) The Move Contributors
+// SPDX-License-Identifier: Apache-2.0
+
+use crate::unit_tests::production_config;
+use move_binary_format::file_format::{
+    empty_module, Bytecode, CodeUnit, FunctionDefinition, FunctionHandle, FunctionHandleIndex,
+    IdentifierIndex, ModuleHandleIndex, Signature, SignatureIndex, SignatureToken,
+    Visibility::Public,
+};
+use move_core_types::{identifier::Identifier, vm_status::StatusCode};
+
+const NUM_LOCALS: u8 = 64;
+const NUM_CALLS: u16 = 77;
+const NUM_FUNCTIONS: u16 = 177;
+
+fn get_nested_vec_type(len: usize) -> SignatureToken {
+    let mut ret = SignatureToken::Bool;
+    for _ in 0..len {
+        ret = SignatureToken::Vector(Box::new(ret));
+    }
+    ret
+}
+
+#[test]
+fn test_large_types() {
+    // See also: github.com/aptos-labs/aptos-core/security/advisories/GHSA-37qw-jfpw-8899
+    let mut m = empty_module();
+
+    m.signatures.push(Signature(
+        std::iter::repeat(SignatureToken::Reference(Box::new(get_nested_vec_type(64))))
+            .take(NUM_LOCALS as usize)
+            .collect(),
+    ));
+
+    m.function_handles.push(FunctionHandle {
+        module: ModuleHandleIndex(0),
+        name: IdentifierIndex(0),
+        parameters: SignatureIndex(0),
+        return_: SignatureIndex(0),
+        type_parameters: vec![],
+    });
+    m.function_defs.push(FunctionDefinition {
+        function: FunctionHandleIndex(0),
+        visibility: Public,
+        is_entry: false,
+        acquires_global_resources: vec![],
+        code: Some(CodeUnit {
+            locals: SignatureIndex(0),
+            code: vec![Bytecode::Call(FunctionHandleIndex(0)), Bytecode::Ret],
+        }),
+    });
+
+    // returns_vecs
+    m.identifiers.push(Identifier::new("returns_vecs").unwrap());
+    m.function_handles.push(FunctionHandle {
+        module: ModuleHandleIndex(0),
+        name: IdentifierIndex(1),
+        parameters: SignatureIndex(0),
+        return_: SignatureIndex(1),
+        type_parameters: vec![],
+    });
+    m.function_defs.push(FunctionDefinition {
+        function: FunctionHandleIndex(1),
+        visibility: Public,
+        is_entry: false,
+        acquires_global_resources: vec![],
+        code: Some(CodeUnit {
+            locals: SignatureIndex(0),
+            code: vec![Bytecode::Call(FunctionHandleIndex(1)), Bytecode::Ret],
+        }),
+    });
+
+    // takes_and_returns_vecs
+    m.identifiers
+        .push(Identifier::new("takes_and_returns_vecs").unwrap());
+    m.function_handles.push(FunctionHandle {
+        module: ModuleHandleIndex(0),
+        name: IdentifierIndex(2),
+        parameters: SignatureIndex(1),
+        return_: SignatureIndex(1),
+        type_parameters: vec![],
+    });
+    m.function_defs.push(FunctionDefinition {
+        function: FunctionHandleIndex(2),
+        visibility: Public,
+        is_entry: false,
+        acquires_global_resources: vec![],
+        code: Some(CodeUnit {
+            locals: SignatureIndex(0),
+            code: vec![Bytecode::Call(FunctionHandleIndex(1)), Bytecode::Ret],
+        }),
+    });
+
+    // takes_vecs
+    m.identifiers.push(Identifier::new("takes_vecs").unwrap());
+    m.function_handles.push(FunctionHandle {
+        module: ModuleHandleIndex(0),
+        name: IdentifierIndex(3),
+        parameters: SignatureIndex(1),
+        return_: SignatureIndex(0),
+        type_parameters: vec![],
+    });
+    m.function_defs.push(FunctionDefinition {
+        function: FunctionHandleIndex(3),
+        visibility: Public,
+        is_entry: false,
+        acquires_global_resources: vec![],
+        code: Some(CodeUnit {
+            locals: SignatureIndex(0),
+            code: vec![Bytecode::Ret],
+        }),
+    });
+
+    // other fcts
+    for i in 0..NUM_FUNCTIONS {
+        m.identifiers
+            .push(Identifier::new(format!("f{}", i)).unwrap());
+        m.function_handles.push(FunctionHandle {
+            module: ModuleHandleIndex(0),
+            name: IdentifierIndex(i + 4),
+            parameters: SignatureIndex(0),
+            return_: SignatureIndex(0),
+            type_parameters: vec![],
+        });
+        m.function_defs.push(FunctionDefinition {
+            function: FunctionHandleIndex(i + 4),
+            visibility: Public,
+            is_entry: false,
+            acquires_global_resources: vec![],
+            code: Some(CodeUnit {
+                locals: SignatureIndex(0),
+                code: vec![],
+            }),
+        });
+
+        let code = &mut m.function_defs[i as usize + 4].code.as_mut().unwrap().code;
+        code.clear();
+        code.push(Bytecode::Call(FunctionHandleIndex(1)));
+        for _ in 0..NUM_CALLS {
+            code.push(Bytecode::Call(FunctionHandleIndex(2)));
+        }
+        code.push(Bytecode::Call(FunctionHandleIndex(3)));
+        code.push(Bytecode::Ret);
+    }
+
+    let result = move_bytecode_verifier::verify_module_with_config_for_test(
+        "test_large_types",
+        &production_config(),
+        &m,
+    );
+    assert_eq!(
+        result.unwrap_err().major_status(),
+        StatusCode::CONSTRAINT_NOT_SATISFIED,
+    );
+}

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/limit_tests.rs

@@ -2,7 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use move_binary_format::file_format::*;
-use move_bytecode_verifier::{limits::LimitsVerifier, verify_module_with_config, VerifierConfig};
+use move_bytecode_verifier::{
+    limits::LimitsVerifier, verify_module_with_config_for_test, VerifierConfig,
+};
 use move_core_types::{
     account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode,
 };
@@ -243,8 +245,8 @@ fn big_vec_unpacks() {
     module.serialize(&mut mvbytes).unwrap();
     let module = CompiledModule::deserialize(&mvbytes).unwrap();
 
-    // run with mainnet aptos config
-    let res = verify_module_with_config(
+    let res = verify_module_with_config_for_test(
+        "big_vec_unpacks",
         &VerifierConfig {
             max_loop_depth: Some(5),
             max_generic_instantiation_length: Some(32),