hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Smart contracts for Kintsu
Other
0 stars 0 forks source link

Missing event emission during ownership transfer in `share_token/lib.rs:transfer_ownership` #3

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xb0619e21af0b222a09552f6abd8ee637475d5f4fe9be4e8450ccb8f4adf6c3a9 Severity: low

Description: Description\

The share_token/lib.rs:transfer_ownership function in the share_token contract is crucial, and emitting events when these actions occur is essential for transparency and informing users about important changes. Adding events for these functions will provide users with a clear record of when the ownership has been transferred to another address.

Attack Scenario\

The absence of events for these critical functions may result in a lack of transparency, making it difficult for users to track important changes in the share_token contract. Emitting events can ensures that users are informed.

Attachments

NA

  1. Proof of Concept (PoC) File

In share_token/lib.rs:transfer_ownership:

#[ink(message)]
        pub fn transfer_ownership(&mut self, new_owner: AccountId) -> Result<(), PSP22Error> {
            if Self::env().caller() != self.owner {
                return Err(PSP22Error::Custom(String::from("Caller is not Owner")));
            }
            self.owner = new_owner;
            Ok(())
        }

Noticed that it is missing event emission.

  1. Revised Code File (Optional)

Consider adding event emission in share_token/lib.rs:transfer_ownership similar to what has been implemented in vault/lib.rs:transfer_role_owner.

bgibers commented 5 months ago

Kintsu was designed to never expose transfer_ownership to anything other than the vault.

0xmahdirostami commented 5 months ago

Also, there is no way for calling 'share_token/lib.rs:transfer_ownership' by the vault and share_token will always be owned and managed by the vault. However, the vault code could be changed, and in further versions, maybe the transfer ownership could be called.