Fixes for issues reported in pentesting audit

aptos-move/aptos-vm/src/gas.rs

@@ -341,7 +341,7 @@ pub(crate) fn check_gas_for_parameters(
         speculative_warn!(
             log_context,
             format!(
-                "[VM] Gas unit error; min {}, submitted {}",
+                "[VM] Gas unit error; max {}, submitted {}",
                 txn_gas_params.max_price_per_gas_unit,
                 txn_gas_metadata.gas_unit_price
             ),

aptos-move/framework/src/natives/aggregator_natives/aggregator_v2.rs

@@ -49,7 +49,7 @@ pub const EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED: u64 = 0x03_0009;
 
 /// The maximum length of the input string for derived string snapshot.
 /// If we want to increase this, we need to modify BITS_FOR_SIZE in types/src/delayed_fields.rs.
-pub const DERIVED_STRING_INPUT_MAX_LENGTH: usize = 1024;
+pub const DERIVED_STRING_INPUT_MAX_LENGTH: usize = 256;
 
 fn get_width_by_type(ty_arg: &Type, error_code_if_incorrect: u64) -> SafeNativeResult<u32> {
     match ty_arg {

aptos-move/framework/supra-framework/sources/automation_registry.move

@@ -822,21 +822,23 @@ module supra_framework::automation_registry {
 
     /// Retrieves specific metadata details of an automation task entry by its task index.
     ///
-    /// 1. `address`                 - The owner of the task.
-    /// 2. `vector<u8>`              - The payload transaction (encoded).
-    /// 3. `u64`                     - The expiry time of the task (timestamp).
-    /// 4. `vector<u8>`              - The hash of the transaction.
-    /// 5. `u64`                     - The maximum gas amount allowed for the task.
-    /// 6. `u64`                     - The gas price cap for executing the task.
-    /// 7. `u64`                     - The automation fee cap for the current epoch.
-    /// 8. `vector<vector<u8>>`      - Auxiliary data related to the task (can be multiple items).
-    /// 9. `u64`                     - The time at which the task was registered (timestamp).
-    /// 10. `u8`                     - The state of the task (e.g., active, cancelled, completed).
-    /// 11. `u64`                    - The locked fee reserved for the next epoch execution.
+    /// 1. `u64`                     - The task index.
+    /// 2. `address`                 - The owner of the task.
+    /// 3. `vector<u8>`              - The payload transaction (encoded).
+    /// 4. `u64`                     - The expiry time of the task (timestamp).
+    /// 5. `vector<u8>`              - The hash of the transaction.
+    /// 6. `u64`                     - The maximum gas amount allowed for the task.
+    /// 7. `u64`                     - The gas price cap for executing the task.
+    /// 8. `u64`                     - The automation fee cap for the current epoch.
+    /// 9. `vector<vector<u8>>`      - Auxiliary data related to the task (can be multiple items).
+    /// 10. `u64`                    - The time at which the task was registered (timestamp).
+    /// 11. `u8`                     - The state of the task (e.g., active, cancelled, completed).
+    /// 12. `u64`                    - The locked fee reserved for the next epoch execution.
     public fun deconstruct_task_metadata(
         task_metadata: &AutomationTaskMetaData
-    ): (address, vector<u8>, u64, vector<u8>, u64, u64, u64, vector<vector<u8>>, u64, u8, u64) {
+    ): (u64, address, vector<u8>, u64, vector<u8>, u64, u64, u64, vector<vector<u8>>, u64, u8, u64) {
         (
+            task_metadata.task_index,
             task_metadata.owner,
             task_metadata.payload_tx,
             task_metadata.expiry_time,

aptos-move/framework/supra-framework/sources/committee_map.move

@@ -188,15 +188,22 @@ module supra_framework::committee_map {
             // f+1, number of nodes in a family committee should be greater than 1
             assert!(num_of_nodes > 1, INVALID_NODE_NUMBERS);
         } else if (committee_type == CLAN) {
-            // 2f+1, number of nodes in a clan committee should be odd and greater than 3
+            // 2f+1, number of nodes in a clan committee should be odd and greater than or equal to 3
             assert!(num_of_nodes >= 3 && num_of_nodes % 2 == 1, INVALID_NODE_NUMBERS);
         } else {
-            // 3f+1, number of nodes in a tribe committee should be in the format of 3f+1 and greater than 4
+            // 3f+1, number of nodes in a tribe committee should be in the format of 3f+1 and greater than or equal to 4
             assert!(num_of_nodes >= 4 && (num_of_nodes - 1) % 3 == 0, INVALID_NODE_NUMBERS);
         };
         committee_type
     }
 
+    /// Ensures removing exactly one member keeps the committee type invariants valid.
+    fun validate_committee_type_after_member_removal(committee: &CommitteeInfo) {
+        let current_num_of_nodes = simple_map::length(&committee.map);
+        assert!(current_num_of_nodes > 0, INVALID_NODE_NUMBERS);
+        validate_committee_type(committee.committee_type, current_num_of_nodes - 1);
+    }
+
     #[view]
     /// Get the committee's node vector and committee type
     public fun get_committee_info(com_store_addr: address, id: u64): (vector<NodeData>, u8) acquires CommitteeInfoStore {
@@ -545,6 +552,27 @@ module supra_framework::committee_map {
         let _acquire = &capability::acquire(owner_signer, &OwnerCap {});
 
         let committee_store = borrow_global_mut<CommitteeInfoStore>(com_store_addr);
+        let event_handler = borrow_global_mut<SupraCommitteeEventHandler>(get_committeeInfo_address(owner_signer));
+
+        // If the node is already associated with another committee, remove the stale entry first.
+        if (simple_map::contains_key(&committee_store.node_to_committee_map, &node_address)) {
+            let old_committee_id = *simple_map::borrow(&committee_store.node_to_committee_map, &node_address);
+            if (old_committee_id != id && simple_map::contains_key(&committee_store.committee_map, &old_committee_id)) {
+                let old_committee = simple_map::borrow_mut(&mut committee_store.committee_map, &old_committee_id);
+                if (does_node_exist(old_committee, node_address)) {
+                    validate_committee_type_after_member_removal(old_committee);
+                    let (_, old_node_info) = simple_map::remove(&mut old_committee.map, &node_address);
+                    emit_event(
+                        &mut event_handler.remove_committee_member,
+                        RemoveCommitteeMemberEvent {
+                            committee_id: old_committee_id,
+                            committee_member: old_node_info
+                        }
+                    )
+                }
+            }
+        };
+
         let committee = simple_map::borrow_mut(&mut committee_store.committee_map, &id);
         let node_info = NodeInfo {
             ip_public_address: copy ip_public_address,
@@ -554,7 +582,6 @@ module supra_framework::committee_map {
             network_port: network_port,
             rpc_port: rpc_port,
         };
-        let event_handler = borrow_global_mut<SupraCommitteeEventHandler>(get_committeeInfo_address(owner_signer));
         if (!does_node_exist(committee, node_address)) {
             emit_event(
                 &mut event_handler.add_committee_member,
@@ -655,6 +682,7 @@ module supra_framework::committee_map {
         let committee_store = borrow_global_mut<CommitteeInfoStore>(com_store_addr);
         let committee = simple_map::borrow_mut(&mut committee_store.committee_map, &id);
         ensure_node_address_exist(committee, node_address);
+        validate_committee_type_after_member_removal(committee);
         let (_, node_info) = simple_map::remove(&mut committee.map, &node_address);
         let event_handler = borrow_global_mut<SupraCommitteeEventHandler>(get_committeeInfo_address(owner_signer));
         emit_event(
@@ -880,6 +908,131 @@ module supra_framework::committee_map {
         );
     }
 
+    #[test(owner_signer = @0xCEFEF)]
+    #[expected_failure(abort_code = INVALID_NODE_NUMBERS, location = Self)]
+    public entry fun test_remove_committee_member_type_validation(
+        owner_signer: &signer
+    ) acquires CommitteeInfoStore, SupraCommitteeEventHandler {
+        set_up_test(owner_signer);
+        let resource_address = account::create_resource_address(&@0xCEFEF, SEED_COMMITTEE);
+        upsert_committee(
+            owner_signer,
+            resource_address,
+            1,
+            vector[@0x1, @0x2],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[123, 124],
+            vector[123, 124],
+            FAMILY
+        );
+        remove_committee_member(owner_signer, resource_address, 1, @0x1);
+    }
+
+    #[test(owner_signer = @0xCEFEF)]
+    #[expected_failure(abort_code = INVALID_NODE_NUMBERS, location = Self)]
+    public entry fun test_transfer_member_type_validation(
+        owner_signer: &signer
+    ) acquires CommitteeInfoStore, SupraCommitteeEventHandler {
+        set_up_test(owner_signer);
+        let resource_address = account::create_resource_address(&@0xCEFEF, SEED_COMMITTEE);
+        upsert_committee(
+            owner_signer,
+            resource_address,
+            1,
+            vector[@0x1, @0x2],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[vector[123], vector[124]],
+            vector[123, 124],
+            vector[123, 124],
+            FAMILY
+        );
+        upsert_committee(
+            owner_signer,
+            resource_address,
+            2,
+            vector[@0x3, @0x4],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[125, 126],
+            vector[125, 126],
+            FAMILY
+        );
+        upsert_committee_member(
+            owner_signer,
+            resource_address,
+            2,
+            @0x1,
+            vector[127],
+            vector[127],
+            vector[127],
+            vector[127],
+            127,
+            127
+        );
+    }
+
+    #[test(owner_signer = @0xCEFEF)]
+    public entry fun test_transfer_committee_member_removes_old_committee_entry(
+        owner_signer: &signer
+    ) acquires CommitteeInfoStore, SupraCommitteeEventHandler {
+        set_up_test(owner_signer);
+        let resource_address = account::create_resource_address(&@0xCEFEF, SEED_COMMITTEE);
+        upsert_committee(
+            owner_signer,
+            resource_address,
+            1,
+            vector[@0x1, @0x2, @0x5],
+            vector[vector[123], vector[124], vector[125]],
+            vector[vector[123], vector[124], vector[125]],
+            vector[vector[123], vector[124], vector[125]],
+            vector[vector[123], vector[124], vector[125]],
+            vector[123, 124, 125],
+            vector[123, 124, 125],
+            1
+        );
+        upsert_committee(
+            owner_signer,
+            resource_address,
+            2,
+            vector[@0x3, @0x4],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[vector[125], vector[126]],
+            vector[125, 126],
+            vector[125, 126],
+            1
+        );
+        upsert_committee_member(
+            owner_signer,
+            resource_address,
+            2,
+            @0x1,
+            vector[127],
+            vector[127],
+            vector[127],
+            vector[127],
+            127,
+            127
+        );
+
+        let (exists_in_old_committee, _) = find_node_in_committee(resource_address, 1, @0x1);
+        assert!(!exists_in_old_committee, 1);
+        assert!(get_committee_id(resource_address, @0x1) == 2, 2);
+        let (exists_in_new_committee, _) = find_node_in_committee(resource_address, 2, @0x1);
+        assert!(exists_in_new_committee, 3);
+
+        // Should succeed because node_to_committee_map and committee maps stay in sync.
+        remove_committee(owner_signer, resource_address, 1);
+    }
+
     #[test(owner_signer = @0xCEFEF)]
     public entry fun test_update_dkg_flag(
         owner_signer: &signer

aptos-move/framework/supra-framework/sources/configs/config_buffer.move

@@ -69,7 +69,7 @@ module supra_framework::config_buffer {
     /// Should only be used at the end of a reconfiguration.
     ///
     /// Typically used in `X::on_new_epoch()` where X is an on-chaon config.
-    public fun extract<T: store>(): T acquires PendingConfigs {
+    public(friend) fun extract<T: store>(): T acquires PendingConfigs {
         let configs = borrow_global_mut<PendingConfigs>(@supra_framework);
         let key = type_info::type_name<T>();
         let (_, value_packed) = simple_map::remove(&mut configs.configs, &key);

aptos-move/framework/supra-framework/sources/fungible_asset.move

@@ -894,6 +894,28 @@ module supra_framework::fungible_asset {
         let metadata_address = object::object_address(&metadata_ref.metadata);
         let mutable_metadata = borrow_global_mut<Metadata>(metadata_address);
 
+        // Validate all provided values before mutating any metadata fields.
+        if (option::is_some(&name)){
+            let new_name = option::borrow(&name);
+            assert!(string::length(new_name) <= MAX_NAME_LENGTH, error::out_of_range(ENAME_TOO_LONG));
+        };
+        if (option::is_some(&symbol)){
+            let new_symbol = option::borrow(&symbol);
+            assert!(string::length(new_symbol) <= MAX_SYMBOL_LENGTH, error::out_of_range(ESYMBOL_TOO_LONG));
+        };
+        if (option::is_some(&decimals)){
+            let new_decimals = option::borrow(&decimals);
+            assert!(*new_decimals <= MAX_DECIMALS, error::out_of_range(EDECIMALS_TOO_LARGE));
+        };
+        if (option::is_some(&icon_uri)){
+            let new_icon_uri = option::borrow(&icon_uri);
+            assert!(string::length(new_icon_uri) <= MAX_URI_LENGTH, error::out_of_range(EURI_TOO_LONG));
+        };
+        if (option::is_some(&project_uri)){
+            let new_project_uri = option::borrow(&project_uri);
+            assert!(string::length(new_project_uri) <= MAX_URI_LENGTH, error::out_of_range(EURI_TOO_LONG));
+        };
+
         if (option::is_some(&name)){
             mutable_metadata.name = option::extract(&mut name);
         };