Security Audit Report for NearOinDao

Report Manifest

Version History

1. Introduction

1.1 About Target Contracts

The target contracts contain a stable coin module. Around it, it also implements other modules, including Staking and Farming. These modules create a positive feedback loop for the stabilization of the stable coin, i.e., USDO.

The repositories that have been audited include NearOinDao ^1

The auditing process is iterative. Specifically, we will further audit the commits that fix the founding issues. If there are new issues, we will continue this process. Thus, there are multiple commit SHA values referred in this report. The commit SHA values before and after the audit are shown in the following.

Before and during the audit

After

1.2 Security Model

To evaluate the risk, we follow the standards or suggestions that are widely adopted by both industry and academy, including OWASP Risk Rating Methodology ^2 and Common Weakness Enumeration ^3. Accordingly, the severity measured in this report are classified into four categories: High, Medium, Low and Undetermined.

2. Findings

In total, we find 22 potential issues in the smart contract. We also have 12 recommendation, as follows:

• High Risk: 19

• Medium Risk: 2

• Low Risk: 1

• Recommendations: 12

The details are provided in the following sections.

2.1 Software Security

2.1.1 Potential Issue 1: Two different attributes for the same usage

Description This issue is introduced in or before Commit-1. Two attributes (i.e., self.cost and self.liquidation_line) represent the same contract state, which is the user’s liquidation line. They are used in different functions of the contract (Listing 2.1 and Listing 2.2). However, self.liquidation_line can be modified with the function set_liquidation_line while self.cost cannot be changed. In this case, if the self.liquidation_line is modified, self.cost keeps the original value. This can influence the logic of the function assert_user_ratio (Listing 2.1).

pub(crate) fn assert_user_ratio(&self) {
        let user_ratio = self.internal_user_ratio(env::predecessor_account_id());
        if user_ratio != 0 {
            assert!(user_ratio >= self.cost, "User ratio less than standard.");
        }
    }

Listing 2.1: assert_user_ratio:lib.rs

// TODO liquidation
    #[payable]
    pub fn liquidation(&mut self, account: AccountId) {
        assert!(self.is_liquidation_paused(), "{}", SYSTEM_PAUSE);
        let ratio = self.internal_user_ratio(account.clone());
        assert!(ratio > 0, "No current pledge");
        assert!(ratio <= self.liquidation_line, "Not at the clearing line");
        ...

Listing 2.2: internal_can_mint_amount:lib.rs

Impact The users' liquidation line is not consistent in the different functions of the contract, which influences the logic of the whole contract.

Suggestion I We can unify the usages of these two attributes when calculating the user's staking ratio and comparing it to the system's liquidation line.

2.1.2 Potential Issue 2: Invalid distribution of the liquidation reward

Description This issue is introduced in or before Commit-4. The liquidation sender's account and the contract owner's account may not be registered (Line 193 and 206 of List 2.3). In this case, when the sender aims to conduct liquidation action, the transaction can not be executed successfully due to the raised exception that accounts are not registered.

pub(crate) fn personal_liquidation_token(&mut self, send_id: AccountId, account_id: AccountId, liquidation_gas: Balance, surplus_token: Balance, liquidation_fee: Balance) {
        //self.owner_id
        let coin_id = ST_NEAR.to_string();
        let mut sys_reward_coin = self.internal_get_reward_coin(coin_id.clone());
        
        let account_reward_key_o = self.get_staker_reward_key(send_id.clone(), coin_id.clone());
        let user_reward_coin_o = self.internal_get_account_reward(send_id.clone(), coin_id.clone());
        
        self.account_reward.insert(
            &account_reward_key_o,
            &UserReward {
                index:  user_reward_coin_o.index,
                reward: user_reward_coin_o.reward.checked_add(liquidation_gas).expect(ERR_ADD),
            },
        );
        
        let account_reward_key_t = self.get_staker_reward_key(account_id.clone(), coin_id.clone());
        let user_reward_coin_t = self.internal_get_account_reward(account_id.clone(), coin_id.clone());

        if surplus_token > 0 {
            self.account_reward.insert(
                &account_reward_key_t,
                &UserReward {
                    index:  user_reward_coin_t.index,
                    reward: user_reward_coin_t.reward.checked_add(surplus_token).expect(ERR_ADD),
                },
            );
        }

        let account_reward_key_s = self.get_staker_reward_key(self.owner_id.clone(), coin_id.clone());
        let user_reward_coin_s = self.internal_get_account_reward(self.owner_id.clone(), coin_id.clone());

        self.account_reward.insert(
            &account_reward_key_s,
            &UserReward {
                index:  user_reward_coin_s.index,
                reward: user_reward_coin_s.reward.checked_add(liquidation_fee).expect(ERR_ADD),
            },
        );
       
        sys_reward_coin.total_reward = sys_reward_coin
            .total_reward
            .checked_add(liquidation_gas).expect(ERR_ADD)
            .checked_add(liquidation_fee).expect(ERR_ADD)
            .checked_add(surplus_token).expect(ERR_ADD);

        self.reward_coins.insert(&coin_id, &sys_reward_coin);
    }

}

Listing 2.3: personal_liquidation_token:reward.rs

Impact Function liquidation cannot be executed successfully due to the raised exception that the accounts are not registered.

Suggestion I Assert the existence of the liquidation sender's account and the contract owner's account at the beginning of function liquidation.

2.1.3 Potential Issue 3: Block_timestamp is saved to the closed_time while opening the system

Description This issue is introduced in or before Commit-3. env::block_time_stamp() should not be saved to the self.closed_time when invoking the function internal_open.

#[private]
    pub fn internal_open(&mut self) {
        self.closed_time = env::block_timestamp();
        self.open_stake();
        self.open_redeem();
        self.open_claim_reward();
        self.open_liquidation();
        self.open_stable();
        log!(
            "{} open sys in {}",
            env::predecessor_account_id(),
            self.closed_time
        );
    }

Listing 2.4: internal_open:esm.rs

Impact The opening time and closed time of the contract is completely wrong. Further updates that depend on the time information can have logic error.

Suggestion I We suggest to create a new contract state called self.opening_time and assigned the env::block_timestamp() to this value while invoking opening the contract.

2.1.4 Potential Issue 4: Contract state is not reverted when the cross function calls are failed

Description This issue is introduced in or before Commit-3. The process of storage_deposit and ft_transfer may fail during the cross contract function calls. We cannot guarantee that the transfer will always be performed correctly. The callback function does not revert the contract state if the call is failed.

#[private]
    pub fn storage_deposit_callback(&mut self) {
        match env::promise_result(0) {
            PromiseResult::NotReady => unreachable!(),
            PromiseResult::Successful(_) => {
                log!("Transfer success");
            }
            PromiseResult::Failed => {
                log!("Transfer failed");
            }
        }
    }

Listing 2.5: storage_deposit_callback:ft.rs

#[private]
    pub fn liquidation_transfer_callback(&mut self) {
        match env::promise_result(0) {
            PromiseResult::NotReady => unreachable!(),
            PromiseResult::Successful(_) => {
                log!("Transfer success");
            }
            PromiseResult::Failed => {
                log!("Transfer failed");
            }
        }
    }

Listing 2.6: liquidation_transfer_callback:ft.rs

Impact Users may loss their assets when transactions failed as the callback function does not revert the contract state.

Suggestion I We need to revert the contract state (when the transfer fails) in the callback function of the cross contract function calls.

2.2 DeFi Security

2.2.1 Potential Issue 5: inject_reward lacks access control

Description This issue is introduced in or before Commit-1. Function inject_reward is public. Anyone can invoke this function to add the balance of the reward in the contract.

pub fn inject_reward(&mut self, amount: U128, reward_coin: AccountId) {
        // self.assert_owner();

        if reward_coin == String::from("NEAR") {
            assert!(
                amount.0 == env::attached_deposit(),
                "Amount not equal transfer_amount"
            );
        }
        ...
    }

Listing 2.7: inject_reward:pool.rs

Impact Anyone can add arbitrary balance on the reward of the contract.

Suggestion I This function should be changed as a private one as it is called internally after receiving the transferred reward.

2.2.2 Potential Issue 6: inject_sp_reward lacks access control

Description This issue is introduced in or before Commit-1. Function inject_sp_reward is public. Anyone can invoke this function to add the balance of the stable pool reward in the contract.

pub fn inject_sp_reward(&mut self, _amount: U128, sender_id: ValidAccountId) {
        self.reward_sp = self.reward_sp + u128::from(_amount);

        log!(
            "{} add sp_reward  {} cur amount{}",
            sender_id,
            u128::from(_amount),
            self.reward_sp
        );
    }

Listing 2.8: inject_sp_reward:stablepool.rs

Impact Anyone can add arbitrary balance on the stable pool reward of the contract.

Suggestion I This function should be changed as a private one as it is called internally after receiving the transferred stable pool reward.

2.2.3 Potential Issue 7: burn_coin lacks access control

Description This issue is introduced in or before Commit-1. Function burn_coin is public. Anyone can invoke this function to burn anyone’s coin.

pub fn burn_coin(&mut self, amount: U128, fee: Balance, sender_id: ValidAccountId) -> Balance{
        assert!(self.is_redeem_paused(), "{}", SYSTEM_PAUSE);
        let sender_id = AccountId::from(sender_id);
        self.assert_is_poked();
        self.accured_token(sender_id.clone());
        ...
    }

Listing 2.9: burn_coin:lib.rs

Impact Anyone can use this function to burn anyone's coin, resulting the loss of users' assets.

Suggestion I This function should be changed as a private one as it is called internally after receiving the transferred stable fee for burning coins.

2.2.4 Potential Issue 8: deposit_token lacks access control

Description This issue is introduced in or before Commit-1. Function deposit_token is public. Anyone can invoke this function to add the balance of their account.

pub fn deposit_token(&mut self, amount: u128, _sender_id: ValidAccountId) {
        self.assert_is_poked();
        assert!(self.is_stake_paused(), "{}", SYSTEM_PAUSE);
        let _amount = u128::from(amount);
        let sender_id = AccountId::from(_sender_id);
        . . .
    }

Listing 2.10: deposit_token:lib.rs

Impact Attackers can invoke this function to add the balance of their account.

Suggestion I This function should be changed as a private one as it is called internally after receiving the deposited tokens.

2.2.5 Potential Issue 9: Oracle lacks the check of time

Description This issue is introduced in or before Commit-1. The function assert_is_poked in oracle.rs only checks whether the value of the token price is zero. This does not makes sense as the token price is keep changing.

pub(crate) fn assert_is_poked(&self) {
        assert!(self.token_price != 0, "Oracle price isn't poked.");
    }

Listing 2.11: assert_is_poked:oracle.rs

Impact This issue affects price oracles. If the token price hasn't been poked for a quiet long time, the assert can still be passed and related transaction can be executed with an outdated price.

Suggestion I The contract should set a valid time period for the poked price.

2.2.6 Potential Issue 10: Inappropriate oracle poke interval time

Description This issue is introduced in or before Commit-3. The constant POKE_INTERVAL_TIME defined in types.rs means 1000 days now. And this time interval seems too long. A reasonable value is required.

pub const POKE_INTERVAL_TIME: u64 = 86_400_000_000_000_000;

Listing 2.12: types.rs

Impact The time interval for poked price is inappropriate.

Suggestion I Reset the interval time for poked price with a reasonable value.

2.2.7 Potential Issue 11: Missing Assert for Oin_Price

Description This issue is introduced in or before Commit-1. This function does not check whether the value of the oin_token price is poked since user's stable fee is calculated by the self.oin_price.

pub fn internal_user_stable(&self, account: AccountId) -> u128 {
        let user_stable = self.account_stable.get(&account).expect("error");
        let allot = self.get_account_allot(account.clone()); 
        let coin = self
            .account_coin
            .get(&account)
            .expect("error")
            .checked_add(allot.0)
            .expect(ERR_ADD);
        let current_block_number = env::block_timestamp().checked_div(INIT_BLOCK_TIME).expect(ERR_DIV);
        user_stable
            .saved_stable
            .checked_add(
                self.stable_fee_rate//16
                    .checked_div(BLOCK_PER_YEAR)
                    .expect(ERR_DIV)
                    .checked_mul(current_block_number as u128 - user_stable.block)
                    .expect(ERR_MUL)
                    .checked_mul(coin)//8
                    .expect(ERR_MUL)
                    .checked_div(self.oin_price)//8
                    .expect(ERR_DIV)
                    .checked_div(ONE_COIN)//8
                    .expect(ERR_DIV),
            )
            .expect(ERR_ADD)
    }

Listing 2.13: internal_user_stable:lib.rs

Impact The outdated OIN price may lead to price manipulation without checking the freshness of the price poked by the oracle.

Suggestion I Add a self.assert_is_poked(); assertion before the calculation of user’s stabel fee.

2.2.8 Potential Issue 12: Users may gain more mining reward with staking token

Description This issue is introduced in or before Commit-1. The claimed reward is not calculated accurately. Function internal_get_saved_reward is called to calculate the user’s specific mining reward from t0 to t1 with the following formula:

Note that account_allot.token is the collateral reward added by other user's liquidation. However, liquidation may happen at any time from t0 to t1. For example, a user deposited 100 token on day0. On day999, liquidation for the other user is triggered so that account_allot.token may increased to 1000.

When the user claims his reward on day1000, the 1000 token resulted from liquidation on day999 should only be counted for mining for one day. However, the contract actually calculate the mining reward for the collateral reward from day0 to day1000.

// TODO[OK] Calculation of reward
    pub(crate) fn internal_get_saved_reward(
        &self,
        staker: AccountId,      
        reward_coin: AccountId, 
    ) -> u128 {
        let reward_coin_ins = self.internal_get_reward_coin(reward_coin.clone());
        let (stake_token_num, _) = self.staker_debt_of(staker.clone());

        if let Some(user_reward) = self
            .account_reward
            .get(&self.get_staker_reward_key(staker.clone(), reward_coin.clone()))
        {
            user_reward
                .reward
                .checked_add(
                    U256::from(
                        reward_coin_ins
                            .index
                            .checked_sub(user_reward.index)
                            .expect(ERR_SUB),
                    )
                    .checked_mul(U256::from(stake_token_num))
                    .expect(ERR_MUL)
                    .checked_div(U256::from(reward_coin_ins.double_scale))
                    .expect(ERR_DIV)
                    .as_u128(),
                )
                .expect(ERR_ADD)
        } else {
            0
        }
    }

Listing 2.14: internal_get_saved_reward:views.rs

pub fn staker_debt_of(&self, staker: AccountId) -> (u128, u128) {
        if let Some(token) = self.account_token.get(&staker) {
            let coin = self.account_coin.get(&staker).expect(ERR_NOT_REGISTER);
            let allot = self.get_account_allot(staker.clone());
            (token + allot.1, coin + allot.0)
        } else {
            (0, 0)
        }
    }

Listing 2.15: staker_debt_of:views.rs

Impact Users may gain extra rewards.

Suggestion I Remove the partition of newly allocated collateral when calculating mining reward. We can make the mining reward only related to the amount of tokens deposited by the user.

2.2.9 Potential Issue 13: Users may pay less stable fee

Description This issue is introduced in or before Commit-1. Suppose one user mints 1000 USDOs on day 0, and the stable_fee_rate at that time is 0.01oin/coin/day. If the user returns back the 1000 USDOs on day100 and the stable fee rate does not change during the past 100 days, the stable fee he needs to pay is 0.01 Oin/coin/day * 1000 Coin * 100 Day = 1000 Oin. However, if the owner set the stable_fee_rate = 0.005 oin/coin/day on day99. In this time, the user only needs to pay 0.005 Oin/Coin/Day * 1000 Coin * 100 Day = 500 Oin. In fact, the accurate fee should be: (0.01 Oin/Coin/Day * 1000 Coin * 99 Day) + (0.005 Oin/Coin/Day * 1000 Coin * 1 Day) = 990 Oin + 5 Oin = 995 Oin.

In this case, the 495 Oin are not required to be paid by users.

// TODO [OK]
    pub fn set_stable_fee_rate(&mut self, fee_rate: U128) {
        self.assert_param_white();
        self.update_stable_index();
        assert!(fee_rate.0 <= INIT_MAX_STABLE_FEE_RATE, "Exceeding the maximum setting");
        self.stable_fee_rate = fee_rate.into();
        log!("Set stable fee rate {}", fee_rate.0);
    }

Listing 2.16: set_stable_fee_rate:dparam.rs

pub fn update_stable_index(&mut self) {
    }

Listing 2.17: update_stable_index:stablefee.rs

Impact Contract users may be charged less for stable fee.

Suggestion I Implement the stable fee’s system index like the calculation of reward_coin in this contract. And make sure that the stable fee’s system index is updated whenever set_stable_fee_rate, liquidation and update_stable_fee is called by contract users.

2.2.10 Potential Issue 14: Unreasonable multi-signed request confirmation rate

Description This issue is introduced in or before Commit-1. The multi-signed request confirmation rate is calculated by the number of multi-signature managers when the request was created. But the number of multi-signature managers may change later. In this case, if the number of managers increases, the request can be confirmed with a low confirmation ratio.

pub(crate) fn is_num_enough(&self, request_id: RequestId) -> bool {
        let request = self.requests.get(&request_id).unwrap();
        let confirmations = self.confirmations.get(&request_id).unwrap();

        let num_confirmrations = request.num_confirm_ratio * (request.mul_white_num);
        log!(
            "confim num is {} num needed is {} ",
            confirmations.len() as u32 * 100,
            num_confirmrations
        );

        (confirmations.len() as u64) * 100 >= num_confirmrations
    }

Listing 2.18: is_num_enough:multisign.rs

pub fn add_request_only(&mut self, request: MultiSigRequest) -> RequestId {
        self.assert_mul_white();
        ...

        let request_added = MultiSigRequestWithSigner {
            signer_pk: env::signer_account_pk(),
            added_timestamp: env::block_timestamp(),
            confirmed_timestamp: 0,
            request: request,
            is_executed: false,
            cool_down: self.request_cooldown,
            mul_white_num: self.mul_white_num(),
            num_confirm_ratio: self.num_confirm_ratio,
        };

        self.requests.insert(&self.request_nonce, &request_added);
        ...
    }

Listing 2.19: add_request_only:multisign.rs

Impact Multi-signed requests may be confirmed with a low confirmation rate as the contract only consider the number of managers when the request is created.

Suggestion I Consider using the number of multi-signed users in the current contract state to calculate the multi-signed request confirmation rate.

2.2.11 Potential Issue 15: Incorrect block number per year

Description This issue is introduced in or before Commit-1. Given that a block is generated every second on the NEAR protocol's mainnet, the generated block number per year should be 31536000 (365days) rather than 31104000 (360days).

pub const BLOCK_PER_YEAR: u128 = 31104000;

Listing 2.20: types.rs

Impact Inaccurate constant for BLOCK_PER_YEAR will make the results of calculations using the constant inconsistent with reality.

Suggestion I Change the BLOCK_PER_YEAR to be 31536000.

2.2.12 Potential Issue 16: Incorrect calculation of the maximum usdo can mint

Description This issue is introduced in or before Commit-1. allot_token.0 represents the allocated debt. While calculating the available mint amount for USDO, the allocated debt should not be counted. Otherwise, a user with very high debt can mint a huge number of USDOs.

pub(crate) fn internal_can_mint_amount(&self, account: AccountId) -> u128 {
        self.assert_is_poked();
        let token = self.account_token.get(&account).expect(ERR_NOT_REGISTER);
        let guarantee = self.guarantee.get(&account).expect(ERR_NOT_REGISTER);
        let allot_token = self.get_account_allot(account.clone());

        let max_usdo = (U256::from(token)
            .checked_add(U256::from(allot_token.1))
            .expect(ERR_ADD))
        .checked_mul(U256::from(self.token_price))
        .expect(ERR_MUL)
        .checked_div(U256::from(self.liquidation_line))
        .expect(ERR_DIV)
        .checked_div(U256::from(INIT_STABLE_INDEX))
        .expect(ERR_DIV)
        .checked_add(U256::from(allot_token.0))
        .expect(ERR_ADD)
        .checked_sub(U256::from(guarantee))
        .unwrap_or(U256::from(0))
        .as_u128();
        
        ...
    }

Listing 2.21: internal_can_mint_amount:lib.rs

Impact Users can mint additional USDOs when invoking the function mint_coin.

Suggestion I The allot_token.0, which represents the allocated debt, should not be counted as the available minted USDOs.

2.2.13 Potential Issue 17: Incorrect handling of user's stable fee

Description This issue is introduced in or before Commit-1. When users invoke the function burn_coin, the stable fee is paid with 'OIN' token rather than 'ST_NEAR'. However, the contract will reduce the balance of the user's staking token, which is not accurate.

pub(crate) fn burn_coin(&mut self, amount: U128, fee: Balance, sender_id: ValidAccountId) -> Balance{
        ...
            assert!(usdo >= amount.into(), "Insufficient amount");
            let token = self.account_token.get(&sender_id.clone()).expect(ERR_NOT_REGISTER);
            self.internal_burn(sender_id.clone(), amount.into());
   
            self.total_token = self.total_token.checked_sub(unpaid_fee.into()).expect(ERR_SUB);
            self.account_token.insert(
                &sender_id.clone(),
                &token.checked_sub(unpaid_fee.into()).expect(ERR_SUB),
            );
        ...
        
    }

Listing 2.22: burn_coin:lib.rs

Impact Users' staking token can be reduced due to the incorrect handling of the user's stable fee.

Suggestion I Use the correct token for paying the stable fees.

2.2.14 Potential Issue 18: Incorrect system ratio

Description This issue is introduced in or before Commit-1. If total_coin = 0, the ratio should be +∞ . Setting it to 0 is incorrect.

pub(crate) fn internal_sys_ratio(&self) -> u128 {
        self.assert_is_poked();
        let token_usd = U256::from(self.total_token)
            .checked_mul(U256::from(self.token_price))
            .expect(ERR_MUL); /* 32 */
        let total_coin = self.total_coin + self.total_guarantee;
        if total_coin == 0 {
            0
        } else {
            token_usd
                .checked_div(U256::from(STAKE_RATIO_BASE))
                .expect(ERR_DIV)
                .checked_div(U256::from(total_coin))
                .expect(ERR_DIV)
                .as_u128()
        }
    }

Listing 2.23: internal_sys_ratio:lib.rs

Impact The system is likely to shut down due to the incorrect ratio.

Suggestion I Change the if condition total_coin = 0 to token_usd = 0.

2.2.15 Potential Issue 19: The number of reward coin can be larger than the upper bound

Description This issue is introduced in or before Commit-1. When there are 20 reward coins now, the assert at line 131 of Listing 2.24 can be passed. In this case, one more reward coin can be added and the total number of rewards coins can be larger than the REWARD_UPPER_BOUND.

pub(crate) fn internal_add_reward_coin(&mut self, coin: RewardCoin) {
        assert!(
            self.reward_coins.len() <= REWARD_UPPER_BOUND,
            "The currency slot has been used up, please modify other currency information as appropriate",
        );

        match self.reward_coins.get(&coin.token) {
            Some(_) => {
                env::panic(b"The current currency has been added, please add a new currency.");
            }
            None => {}
        }
        self.reward_coins.insert(&coin.token, &coin);

        log!(
            "{} add the RewardCoin=> {:?}",
            env::predecessor_account_id(),
            coin
        )
    }

Listing 2.24: internal_add_reward_coin:pool.rs

Impact The available added number of reward coins is conflicted with the design of the system.

Suggestion I Change the assert into self.reward_coins.len() < REWARD_UPPER_BOUND.

2.2.16 Potential Issue 20: Users in different privileges use the same white list

Description This issue is introduced in or before Commit-1. Functions assert_param_white, assert_white, assert_esm_white,assert_oracle_white are used for different privileges. However, they share the same whitelist.

pub(crate) fn assert_esm_white(&self) {
        self.assert_white()
    }

Listing 2.25: assert_esm_white:esm.rs

pub(crate) fn assert_param_white(&self) {
        self.assert_white();
    }

Listing 2.26: assert_param_white:dparam.rs

pub(crate) fn assert_oracle_white(&self) {
        self.assert_white();
    }

Listing 2.27: assert_oracle_white:oracle.rs

Impact Users in different privilege share the same white list.

Suggestion I Implement different white lists for users with different privileges.

2.2.17 Potential Issue 21: burn_coin does not check the token type

Description This issue is introduced in or before Commit-1. Functions burn_coin does not check the token type. In this case, attackers can transfer arbitrary tokens with specified amount for paying the stable fee.

pub fn burn_coin(&mut self, amount: U128, fee: Balance, sender_id: ValidAccountId) -> Balance{
        assert!(self.is_redeem_paused(), "{}", SYSTEM_PAUSE);
        let sender_id = AccountId::from(sender_id);

Listing 2.28: assert_esm_white:esm.rs

Impact Users do not need to pay Oin token. Instead, they can pay the stable fee by transfer arbitrary token with the required amount.

Suggestion I Check the address of the received token.

2.2.18 Potential Issue 22: Reward coin's total_reward can be modified by multi-Signature managers

Description This issue is introduced in or before Commit-3. Function inject_reward is decorated with #[private]. Therefore, multi-signature managers can invoke this function through multi-signature requests and add arbitrary amount on the total reward without injecting reward.

#[payable]
    #[private]
    pub fn inject_reward(&mut self, amount: U128, reward_coin: AccountId) {
        // self.assert_owner();

        if reward_coin == String::from("NEAR") {
            assert!(
                amount.0 == env::attached_deposit(),
                "Amount not equal transfer_amount"
            );
        }

        if let Some(reward_coin_ins) = self.get_reward_coin(reward_coin.clone()) {
            let mut reward_coin_ins = reward_coin_ins;
            reward_coin_ins.total_reward = reward_coin_ins
                .total_reward
                .checked_add(amount.into())
                .expect(ERR_SUB);
            self.reward_coins.insert(&reward_coin, &reward_coin_ins);

            if reward_coin == String::from("NEAR") {
            
            } else {
                log!("Transfer is not required for post-processing");
            }
        } else {
            env::panic(b"No the reward coin.");
        }
    }

Listing 2.29: ainject_reward:pool.rs

Suggestion I Remove the decorator #[private], and change the visibility of the function inject_reward to be private.

2.3 Additional Recommendation

2.3.1 Redundant assertion

Description This issue is introduced in or before Commit-2. Function inject_reward should only be called by ft_on_transfer internally. The address of the reward coin is checked in ft_on_transfer. In this case, we do not need to check the name of reward coin at the beginning of the function inject_reward.

#[payable]
    #[private]
    pub  fn inject_reward(&mut self, amount: U128, reward_coin: AccountId) {
        // self.assert_owner();

        if reward_coin == String::from("NEAR") {
            assert!(
                amount.0 == env::attached_deposit(),
                "Amount not equal transfer_amount"
            );
        }

    ...
    }

Listing 2.30: inject_reward:pool.rs

    pub fn ft_on_transfer(
        &mut self,
        sender_id: ValidAccountId,
        amount: U128,
        msg: String, /* token */
    ) -> PromiseOrValue<U128> {
    ...
            FtOnTransferArgs::InjectReward => {
                assert_eq!(sender_id.to_string(), self.owner_id, "ERR_NOT_ALLOWED");

                assert!(
                    self.reward_coins.get(&token_account_id).is_some(),
                    "Invalid reward coin"
                );

                self.inject_reward(amount, token_account_id);
                amount_return = 0;
            }
    ...
    }

Listing 2.31: ft_on_transfer:lib.rs

Suggestion I Remove check on the name of reward coin in inject_reward.

2.3.2 Repeated assertion for user's liquidation ratio

Description This issue is introduced in or before Commit-1. The liquidation line is already taken into consideration in function internal_avaliable_token, so there is no need to check whether the user_ratio’s reaches the liquidation line later.

#[payable]
    pub fn withdraw_token(&mut self, amount: U128) {
        assert!(self.is_stake_paused(), "{}", SYSTEM_PAUSE);
        let mut amount = amount.0;

        let token = self.internal_avaliable_token(env::predecessor_account_id());
        let debt = self.get_dept(env::predecessor_account_id());

        log!("token :{} amount: {}", token, amount);
        assert!(token >= amount, "Insufficient avaliable token.");
        if debt.0 - debt.2 == 0 {
            if token - amount < self._min_amount_token() {
                amount = token;
            }
        } else {
            self.assert_user_ratio();
            if token - amount < self._min_amount_token() {
                env::panic(b"Please return all coins first");
            }
        }

Listing 2.32: withdraw_token:lib.rs

Suggestion I Remove the redundant assertion in Line 559 of Listing 2.32.

2.3.3 Redundant whitelist check

Description This issue is introduced in or before Commit-1. Function set_reward_speed invoke the function assert_param_white to check the privilege. Meanwhile, the internal_set_reward_speed, which is called by set_reward_speed, invoke the assert_white again. assert_white has the same whitelist as the assert_param_white.

pub fn set_reward_speed(&mut self, reward_coin: AccountId, speed: U128) {
        self.assert_param_white();
        self.internal_set_reward_speed(reward_coin, speed);
    }

Listing 2.33: set_reward_speed:dparam.rs

pub(crate) fn internal_set_reward_speed(&mut self, reward_coin: AccountId, speed: U128) {
        self.assert_white();
        self.update_index();
        . . .
    }

Listing 2.34: internal_set_reward_speed:pool.rs

Suggestion I Remove assert_white inside the function internal_set_reward_speed.

2.3.4 Unused function

Description This issue is introduced in or before Commit-3. Function on_inject_reward is not used by any other functions. Thus, it can be removed.

#[private]
    pub fn on_inject_reward(&mut self, reward_coin: AccountId, amount: U128) {
        match env::promise_result(0) {
            PromiseResult::NotReady => unreachable!(),
            PromiseResult::Successful(_) => {}
            PromiseResult::Failed => {
                let mut reward_coin_ins = self.internal_get_reward_coin(reward_coin.clone());
                reward_coin_ins.total_reward = reward_coin_ins
                    .total_reward
                    .checked_sub(amount.into())
                    .expect(ERR_ADD);
                self.reward_coins.insert(&reward_coin, &reward_coin_ins);
            }
        };
    }

Listing 2.35: on_inject_reward:pool.rs

Suggestion I Remove the function on_inject_reward.

2.3.5 Redundant Code

Description This issue is introduced in or before Commit-3. Function account_allot.get() is used to get the allocated reward and debt. Inside the function set_account_allot, the invocation of this function is not required.

pub(crate) fn set_account_allot(&mut self,account_id: AccountId){
        //Update [personally assigned debt, personally assigned pledge] to system value
        let (allot_debt, allot_token) = self.get_account_allot(account_id.clone());
        let token = self.account_token.get(&account_id).expect(ERR_NOT_REGISTER);
        let coin = self.account_coin.get(&account_id).expect(ERR_NOT_REGISTER);

        self.account_allot.get(&account_id);

        self.account_allot.insert(
            &account_id, 
            &AccountAllot{
                account_allot_debt: self.sys_allot_debt,
                account_allot_token: self.sys_allot_token,
            }
        );
        self.account_coin.insert(&account_id, &coin.checked_add(allot_debt).expect(ERR_ADD));
        self.account_token.insert(&account_id, &token.checked_add(allot_token).expect(ERR_ADD));       
    }

Listing 2.36: set_account_allot:allot.rs

Suggestion I Remove the invocation account_allot.get() at line 42.

2.3.6 The function name and the implementation is opposite

Description This issue is introduced in or before Commit-3. Function is_stake_paused, is_redeem_paused, is_claim_reward_paused,is_liquidation_paused,is_stable_paused are defined to represent whether the function is paused or not. However, when the specific attribute is live, it returns True.

// TODO [OK]
    pub(crate) fn is_stake_paused(&self) -> bool {
        self.stake_live == 1
    }

    // TODO [OK]
    pub(crate) fn is_redeem_paused(&self) -> bool {
        self.redeem_live == 1
    }

    // TODO [OK]
    pub(crate) fn is_claim_reward_paused(&self) -> bool {
        self.claim_live == 1
    }

    // TODO [OK]
    pub(crate) fn is_liquidation_paused(&self) -> bool {
        self.liquidation_live == 1
    }

    // TODO [OK]
    pub(crate) fn is_stable_paused(&self) -> bool {
        self.stable_live == 1
    }

Listing 2.37: is_{stake|redeem|claim_reward|liquidation|stable}_paused:esm.rs

Suggestion I Change the function name of is_{stake|redeem|claim_reward|liquidation|stable}_paused into is_{stake|redeem|claim_reward|liquidation|stable}_live

2.3.7 Redundant Code

Description This issue is introduced in or before Commit-3. Function update_stable_fee is used to update the required stable fees. Stable fees is not related to the staked tokens. Thus, changing the balance of token for users does not need to update the stable fees.

pub(crate) fn deposit_token(&mut self, _amount: u128, _sender_id: ValidAccountId) {
        self.assert_is_poked();
        assert!(self.is_stake_paused(), "{}", SYSTEM_PAUSE);
        let sender_id = AccountId::from(_sender_id);
        assert!(_amount > 0, "Deposit token amount must greater than zero.");

        if let Some(0) = self.guarantee.get(&sender_id) {
            assert!(
                _amount >= self._min_amount_token(),
                "Deposit token amount must greater the minimum deposit token."
            );
        }
        self.update_personal_token(sender_id.clone());
        self.update_stable_fee(sender_id.clone());
        self.set_account_allot(sender_id.clone());
        . . .
    }

Listing 2.38: deposit_token:lib.rs

Suggestion I Remove the invocation update_stable_fee at line 344.

2.3.8 The calculation precision can be enhanced

Description This issue is introduced in or before Commit-3. Function internal_user_stable aims to calculate the stable fee. The calculation precision can be enhanced by conducting multiplication before division.

pub(crate) fn update_stable_fee(&mut self, account: AccountId) {
        if let Some(mut user_stable) = self.account_stable.get(&account) {
            let allot = self.get_account_allot(account.clone());
            let debt = allot.0;
            let current_block_number = self.to_nano( env::block_timestamp()) as u128;

            let coin = self.account_coin.get(&account).expect(ERR_NOT_REGISTER).checked_add(debt).expect(ERR_ADD);
            let delta_block = current_block_number.checked_sub(user_stable.block).expect(ERR_SUB);
            if delta_block > 0 && coin > 0 {
                let fee = self.stable_fee_rate//16
                        .checked_mul(delta_block).expect(ERR_MUL)
                        .checked_mul(coin).expect(ERR_MUL)//8
                        .checked_div(BLOCK_PER_YEAR).expect(ERR_DIV)
                        .checked_div(self.oin_price).expect(ERR_DIV)//8
                        .checked_div(ONE_COIN).expect(ERR_DIV);//8
                        
                self.saved_stable = self.saved_stable
                        .checked_add(fee).expect(ERR_ADD);

                user_stable.saved_stable = user_stable.saved_stable
                        .checked_add(fee).expect(ERR_ADD); 
            }
            
            user_stable.block = current_block_number;
            self.account_stable.insert(&account, &user_stable);    
            log!("Current stabilization fee: {:?}",self.account_stable.get(&account));
        } else {
            env::panic(b"Not register")
        }
    }

Listing 2.39: update_stable_fee:stablefee.rs

Suggestion I Conduct the multiplication before division for the calculation from line 25 to line 30.

2.3.9 System may not record previously poked price

Description This issue is introduced in or before Commit-1. The function is not implemented correctly. System may not record poked price as the number of total tokens deposited in the contract is greater than 0 in most cases.

pub fn poke(&mut self, token_price: U128) {
    ...
       if self.total_token > 0 {
           if self.internal_sys_ratio() <= INIT_MIN_RATIO_LINE {
                self.internal_shutdown();
           }
       }else {
            log!(
                "{} poke price {} successfully.",
                env::predecessor_account_id(),
                token_price.0
            );
        }
    }

Listing 2.40: poke:oracle.rs

Suggestion I Recording the behavior of poking token price should not be influenced by the number of deposited tokens in the contract.

2.3.10 Discontinuous distribution of collateral token in liquidation

Description This issue is introduced in or before Commit-4. When the user's staking ratio is larger or equal than 108.5%, users have to pay the liquidation_fee, which owns 2% of the allot_debt. However, if the user's staking ratio is less than 108.5%, he/she does not need to pay the liquidation fee. This result in the fact that user with larger staking ratio may allot less staking token to the pool after liquidation.

#[payable]
    pub fn liquidation(&mut self, account: AccountId) {
        ...
        if ratio >= INIT_NO_LIQUIDATION_FEE_RATE {
            liquidation_fee = _allot_debt
                            .checked_mul(self.liquidation_fee_ratio).expect(ERR_MUL)
                            .checked_mul(STAKE_RATIO_BASE).expect(ERR_MUL)//16
                            .checked_div(self.token_price).expect(ERR_DIV);
        }else{
            allot_ratio = ratio
                .checked_sub(self.gas_compensation_ratio).expect(ERR_SUB)
                .checked_add(1).expect(ERR_ADD);
        }
        ...

Listing 2.41: liquidation:lib.rs

Suggestion I For user whose staking ratio is between 108.5% to 110.5%, the liquidation fee is suggested to be (staking ratio - 108.5%).

2.3.11 Optimization of calculation precision is not necessary

Description This issue is introduced in or before Commit-4. Adding 1 in line 832 in listing 2.42 cannot increase the calculation precision as self.gas_compensation_ratio is rather large.

#[payable]
    pub fn liquidation(&mut self, account: AccountId) {
        ...
        if ratio >= INIT_NO_LIQUIDATION_FEE_RATE {
            liquidation_fee = _allot_debt
                            .checked_mul(self.liquidation_fee_ratio).expect(ERR_MUL)
                            .checked_mul(STAKE_RATIO_BASE).expect(ERR_MUL)//16
                            .checked_div(self.token_price).expect(ERR_DIV);
        }else{
            allot_ratio = ratio
                .checked_sub(self.gas_compensation_ratio).expect(ERR_SUB)
                .checked_add(1).expect(ERR_ADD);
        }
        ...

Listing 2.42: liquidation:lib.rs

Suggestion I Remove the added "1" in line 831 of listing 2.42.

2.3.12 The Risk of Centralized Design

Status Acknowledged

Description Description The project has a highly centralized design. The contract owner has very high privilege that can add/delete the multi-signature managers and can withdraw the liquidation fee and reward, etc. Such mechanism is absolutely centralized, which has a complete control power over all tokens. We highly suggest that the project owner should enforce security mechanisms to protect the private keys of the contract owner to manage the contracts.

3. Notices and Remarks

3.1 Disclaimer

This audit report does not constitute investment advice or a personal recommendation. It does not consider, and should not be interpreted as considering or having any bearing on, the potential economics of a token, token sale or any other product, service or other asset. Any entity should not rely on this report in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset.

This audit report is not an endorsement of any particular project or team, and the report does not guarantee the security of any particular project. This audit does not give any warranties on discovering all security issues of the smart contracts, i.e., the evaluation result does not guarantee the nonexistence of any further findings of security issues. As one audit cannot be considered comprehensive, we always recommend proceeding with independent audits and a public bug bounty program to ensure the security of smart contracts.

The scope of this audit is limited to the code mentioned in Section 1.1. Unless explicitly specified, the security of the language itself (e.g., the Rust language), the underlying compiling toolchain and the computing infrastructure are out of the scope.

3.2 Procedure of Auditing

We perform the audit according to the following procedure.

• Vulnerability Detection We first scan smart contracts with automatic code analyzers, and then manually verify (reject or confirm) the issues reported by them.

• Semantic Analysis We study the business logic of smart contracts and conduct further investigation on the possible vulnerabilities using an automatic fuzzing tool (developed by our research team). We also manually analyze possible attack scenarios with independent auditors to cross-check the result.

• Recommendation We provide some useful advice to developers from the perspective of good programming practice, including gas optimization, code style, and etc.

We show the main concrete checkpoints in the following.

3.2.1 Software Security

• Reentrancy

• DoS

• Access control

• Data handling and data flow

• Exception handling

• Untrusted external call and control flow

• Initialization consistency

• Events operation

• Error-prone randomness

• Improper use of the proxy system

3.2.2 DeFi Security

• Semantic consistency

• Functionality consistency

• Access control

• Business logic

• Token operation

• Emergency mechanism

• Oracle security

• Whitelist and blacklist

• Economic impact

• Batch transfer

3.2.3 NFT Security

• Duplicated item

• Verification of the token receiver

• Off-chain metadata security

3.2.4 Additional Recommendation

• Gas optimization

• Code quality and style

::: Note The previous checkpoints are the main ones. We may use more checkpoints during the auditing process according to the functionality of the project. :::