update_emergency_council_7_D_0_C_1_C_58() updates nft manager instead of emergency council

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L1121

Vulnerability details

Impact

Inside of lib.rs, there is a function update_emergency_council_7_D_0_C_1_C_58() that is needed to update the emergency council that can disable the pools. However, in the current implementation, nft_manager is updated instead.

Proof of Concept

This is the current functionality of update_emergency_council_7_D_0_C_1_C_58():

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L1111-1124

pub fn update_emergency_council_7_D_0_C_1_C_58(
        &mut self,
        manager: Address,
    ) -> Result<(), Revert> {
        assert_eq_or!(
            msg::sender(),
            self.seawater_admin.get(),
            Error::SeawaterAdminOnly
        );

        self.nft_manager.set(manager);

        Ok(())
    }

As you can see, the function updates nft_manager contract instead of emergency_council that is needed to be updated. Above this function there is another function that updates nft_manager:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L1097-1107

  pub fn update_nft_manager_9_B_D_F_41_F_6(&mut self, manager: Address) -> Result<(), Revert> {
        assert_eq_or!(
            msg::sender(),
            self.seawater_admin.get(),
            Error::SeawaterAdminOnly
        );

        self.nft_manager.set(manager);

        Ok(())
    }

As you can see here, in both of the functions nft_manager is updated which is an unexpected behavior and the contract cannot update the emergency_council that handles emergency situations:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L117-118

 // address that's able to activate and disable emergency mode functionality
    emergency_council: StorageAddress,

Tools Used

Manual review.

Recommended Mitigation Steps

Change update_emergency_council_7_D_0_C_1_C_58() to update emergency_council

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as selected for report

[c4-judge]

alex-ppg marked the issue as satisfactory

[alex-ppg]

The Warden and its duplicates have correctly identified that the mechanism exposed for updating the emergency_council will incorrectly update the nft_manager instead.

I initially wished to retain a medium risk severity rating for this vulnerability due to how the emergency_council is configured during the contract's initialization and its value changing being considered a rare event, however, a different highly sensitive variable is altered instead incorrectly (nft_manager) which would have significant consequences to the system temporarily.

Based on the above, I believe that a high-risk rating is appropriate due to the unexpected effects invocation of the function would result in.