Transfer role is single step transfer

Github username: -- Twitter username: -- Submission hash (on-chain): 0x8c3669b801fc78815bcb5775dcc1837dbac88c40a24ab12c9fb144c0a6a13e98 Severity: low

Description: Description\

The vault has an owner which have the role_owner. This role can do critical function call such as withdraw fees, set minimum stake, and upgrade the Vault.

Currently, process for transferring ownership role, as managed by the 'transfer_role_owner' function, lacks a crucial two-step confirmation mechanism. Typically, when transferring any administrative role, it's imperative to ensure a secure and verified process, often involving both assigning the role and confirming the transfer. Current existing code fails to follow this.

To mitigate this, revise the 'transfer_role_owner' process to align with industry best practices for role transfers. Implementing a two-step confirmation mechanism would significantly enhance the security posture of the system, mitigating the risk of unauthorized ownership transfers or inadvertent changes to critical functions. By requiring both assignment and confirmation steps, the process introduces an additional layer of verification, reducing the likelihood of errors or malicious activity.

File: lib.rs
590:         /// Transfers ownership to a new account
591:         ///
592:         /// Caller must have the owner role (`role_owner`)
593:         #[ink(message)]
594:         pub fn transfer_role_owner(&mut self, new_account: AccountId) -> Result<(), VaultError> {
595:             let caller = Self::env().caller();
596: 
597:             if caller != self.data.role_owner {
598:                 return Err(VaultError::InvalidPermissions);
599:             }
600:             if self.data.role_owner == new_account {
601:                 return Err(VaultError::NoChange);
602:             }
603: 
604: =>        self.data.role_owner = new_account;
605: 
606:             Self::emit_event(
607:                 Self::env(),
608:                 Event::OwnershipTransferred(OwnershipTransferred {
609:                     new_account,
610:                 }),
611:             );
612: 
613:             Ok(())
614:         }

as we can see from above code, when the transfer_role_owner is executed with a wrong AccountId, it will pass through instantly without any confirmation if the Account is valid or not.

Attack Scenario

When current owner transfer to wrong account, it will effective immediately, and it will brick or lock up the vault. This is a low issue, because the potential occurance is very low, but if it's executed wrongly, it will raise a critical issue.

Attachments

Proof of Concept (PoC) File
Revised Code File (Optional)

Recommendation

Consider to implement two step transfer role to prevent any potential issue, like losing control of the vault

hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Transfer role is single step transfer #7