code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

DoS: Admin can set penalties to 0 in `updatePenalties`, leading to users being unable to get tokens back #225

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L136 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L195-L198

Vulnerability details

Impact

Users can call rageQuit, and apply penalties for unvested tokens at any time. But admin can trigger DoS, leading to users being unable to quit.

Proof of Concept

In rageQuit, it doesn’t have the whenNotPaused modifier, so any users can call rageQuit at any time. It uses getRageQuitAmounts to calculate totalToUser and penalty. But in getRageQuitAmounts L195-L198: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L195-L198

    uint256 totalToUser = totalVested +
      ((threeMonthLock - threeMonthVested) / THREE_MONTH_PENALTY) +
      ((sixMonthLock - sixMonthVested) / SIX_MONTH_PENALTY) +
      ((twelveMonthLock - twelveMonthVested) / TWELVE_MONTH_PENALTY);

If the admin set THREE_MONTH_PENALTY to 0, the transaction will be reverted because of division by zero. A malicious/compromised owner can deny everyone to get tokens back.

Tools Used

None

Recommended Mitigation Steps

Check the new penalty value should not be 0 in updatePenalties.

nneverlander commented 2 years ago

Assessment low. Fixed in https://github.com/infinitydotxyz/exchange-contracts-v2/commit/4d1ac0af44b0cd55e7eedc95931cca9229602db0

nneverlander commented 2 years ago

https://github.com/code-423n4/2022-06-infinity-findings/issues/117

HardlyDifficult commented 2 years ago

Lowering risk since this is an admin function and they could update again if the value was set in error. Merging with the warden's QA report #232