code-423n4 / 2024-08-superposition-findings

0 stars 0 forks source link

Emergency Council Update Function Incorrectly Modifies NFT Manager #103

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

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

Vulnerability details

The update_emergency_council_7_D_0_C_1_C_58() function is intended to update the emergency council address, which is a critical security role. However, the function actually updates the nft_manager instead of the emergency_council:

#[allow(non_snake_case)]
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(())
}

This error leads to a situation where the emergency council cannot be updated as intended, and the NFT manager can be changed unexpectedly.

Impact

  1. The emergency council cannot be updated, potentially leaving an outdated or compromised address in control of emergency functions.
  2. The NFT manager can be unintentionally changed, giving unexpected privileges to transfer NFTs to an address meant for the emergency council.

Proof of Concept

  1. The seawater_admin calls update_emergency_council_7_D_0_C_1_C_58() with a new address, intending to update the emergency council.
  2. The function executes successfully, but instead of updating emergency_council, it updates nft_manager.
  3. The emergency_council remains unchanged, while nft_manager is now set to the new address.

Tools Used

Manual review

Recommended Mitigation Steps

The function should be corrected to update the emergency_council instead of the nft_manager.

Assessed type

Other

c4-judge commented 3 weeks ago

alex-ppg changed the severity to 3 (High Risk)

c4-judge commented 3 weeks ago

alex-ppg marked the issue as satisfactory