hats-finance / AlephZeroAMM-0x0d88a9ece90994ecb3ba704730819d71c139f60f

Apache License 2.0
1 stars 0 forks source link

Self not declared as first parameter in functions #12

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x5f3db33b785c1d1ebccaa80da95e44d793c070768c8ae8793fbce289bf77cf68 Severity: medium

Description: Description\ &mut self or &self, parameter is not included for method of rewards_per_share_in_time_interval and rewards_earned_by_shares in farm contract making them unable to access.

Attack Scenario\

As per Ink Official docs: “the function template included self as the first parameter of the contract functions. It is through self that you gain access to all your contract functions and storage items.”

self parameter absence will make functions of rewards_per_share_in_time_interval and rewards_earned_by_shares unable to access in either case; whether they are used for reading or writing purpose.

Attachments

https://github.com/Cardinal-Cryptography/common-amm/blob/0a7264d707aea51b559a1bf94448681b59660f6a/farm/contract/lib.rs#L423

https://github.com/Cardinal-Cryptography/common-amm/blob/0a7264d707aea51b559a1bf94448681b59660f6a/farm/contract/lib.rs#L446

  1. Proof of Concept (PoC) File

No #[ink(message)] attribute defined for rewards_per_share_in_time_interval and rewards_earned_by_shares methods as they are clearly public.

rewards_per_share_in_time_interval function

    pub fn rewards_per_share_in_time_interval(
        reward_rate: u128,
        total_shares: u128,
        from_timestamp: u128,
        to_timestamp: u128,
    ) -> Result<U256, MathError> {
        if total_shares == 0 || from_timestamp >= to_timestamp {
            return Ok(0.into());
        }
        let time_delta = to_timestamp
            .checked_sub(from_timestamp)
            .ok_or(MathError::Underflow)?;
        casted_mul(reward_rate, time_delta)
            .checked_mul(U256::from(SCALING_FACTOR))
            .ok_or(MathError::Overflow(1))?
            .checked_div(U256::from(total_shares))
            .ok_or(MathError::DivByZero(1))
    }

and rewards_earned_by_shares function.

    pub fn rewards_earned_by_shares(
        shares: u128,
        rewards_per_share: U256,
    ) -> Result<u128, MathError> {
        rewards_per_share
            .checked_mul(U256::from(shares))
            .ok_or(MathError::Overflow(2))?
            .checked_div(U256::from(SCALING_FACTOR))
            .ok_or(MathError::DivByZero(2))?
            .try_into()
            .map_err(|_| MathError::CastOverflow)
    }
  1. Revised Code File (Optional)

Pass &self as first parameter for both the rewards_per_share_in_time_interval and rewards_earned_by_shares functions to gain access to them.

    pub fn rewards_per_share_in_time_interval(
+     &self,   
        reward_rate: u128,
        total_shares: u128,
        from_timestamp: u128,
        to_timestamp: u128,
    ) -> Result<U256, MathError> {
        if total_shares == 0 || from_timestamp >= to_timestamp {
            return Ok(0.into());
        }
        let time_delta = to_timestamp
            .checked_sub(from_timestamp)
            .ok_or(MathError::Underflow)?;
        casted_mul(reward_rate, time_delta)
            .checked_mul(U256::from(SCALING_FACTOR))
            .ok_or(MathError::Overflow(1))?
            .checked_div(U256::from(total_shares))
            .ok_or(MathError::DivByZero(1))
    }

If the functions are meant for modifying state pass &mut self parameter.

    pub fn rewards_earned_by_shares(
+   &mut self,   
     shares: u128,
        rewards_per_share: U256,
    ) -> Result<u128, MathError> {
        rewards_per_share
            .checked_mul(U256::from(shares))
            .ok_or(MathError::Overflow(2))?
            .checked_div(U256::from(SCALING_FACTOR))
            .ok_or(MathError::DivByZero(2))?
            .try_into()
            .map_err(|_| MathError::CastOverflow)
    }
deuszx commented 10 months ago

These are pure functions, not methods of the contract (so also no #[ink(message)] attribute needed), i.e. they don't operate on any "self state", and so they don't need the parameter.

deuszx commented 10 months ago

Thank you for participation. After carefully reviewing the submission we've concluded that this issue is INVALID.

We hope you participate in the future audits of ink!.