[security] Cherry-picking recent security fixes from Aptos (move-language#950)

* [verifier] limit the number of back edges

* [verifier] fix incorrect error code for per-module back edge limit check

* copyloc-pop test (move-language#54)

* [gas] allow natives to read the gas balance

* [bytecode-verifier] Add metering logic and apply to absint based analysis (move-language#58)

This adds a simple meter to the bytecode verifier which counts the number of abstract operations performed and can enforce a limit. The meter is for now only connected to locals and reference analysis, but plumped to all phases of the verifier so they can easily make use of it.

A set of test cases have been added which exercise the new meter for a number of known pathological cases.

PR history:

- Add metering in type safety, to capture cost of very large types. This reduces timing of large_type_test to 1/4
- Adjusting max metering units upwards and adding a new sample which needs it
- Addressing reviewer comments
- Add links to security advisories, and verify that all are covered.
- Switching metering granularity from function to module.
- Adding non-linear growing penalty to using input refs x output refs relations (bicycles), for dealing better with `test_bicliques`. Adding printing size in benchmarks.

* [bytecode verifer] Adjust metering to decrease runtime of some tests. (move-language#62)

Specifically the test below now runs in 1/2 of the time. This adjustment appeard useful because the overall time allocation had to be increased to 8000 million units in production. Adjusted this as the default here too.

```
--> test_merge_state: verification time: 59.414ms, result: CONSTRAINT_NOT_SATISFIED, size: 63kb
```

Also adjusts the default to what aptos uses now in production.

* [bytecode verifier] Meter type instantiations (move-language#64)

Instead of just metering size of types on the operand stack, also meter size of type instantiations in calls and other places. This e.g. capture the size of types in calls like `f<T>()`, where the type does not appear on the operand stack.

---------

Co-authored-by: Victor Gao <[email protected]>
Co-authored-by: Teng Zhang <[email protected]>

Author: 3 people

Cargo.lock

@@ -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-binary-format/src/control_flow_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-borrow-graph/src/graph.rs

@@ -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/Cargo.toml

@@ -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/METER_TESTING.md

@@ -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/binary_samples.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/catch_unwind.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/code_unit_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/constants_tests.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/control_flow_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),