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

4 stars 0 forks source link

Continuous staking within same duration prolongs staking token vesting time #350

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L67-L77

Vulnerability details

Impact

Infinity tokens can be staked for a specified duration. Only after the duration ends, tokens are fully vested and can be fully unstaked (they can be unstaked earlier but the user would have to pay penalties).

If a user stakes multiple times with the same specified duration (e.g. 3 months), each and every staking event prolongs the vesting time of the previously staked tokens.

Proof of Concept

User staked tokens with a specified duration of 3 months. 3 months later, shortly before the duration would end and the tokens would be fully vested, the user decides to stake even more tokens again for a duration of 3 months. However, the previously staked tokens can not be unstaked now as the locking duration was prolonged. The user has to wait further 3 months.

InfinityStaker.InfinityStaker

function stake(uint256 amount, Duration duration) external override nonReentrant whenNotPaused {
    require(amount != 0, 'stake amount cant be 0');
    require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake');
    // update storage
    userstakedAmounts[msg.sender][duration].amount += amount;
    userstakedAmounts[msg.sender][duration].timestamp = block.timestamp; // @audit-info timestamp of last staking event is overwritten
    // perform transfer
    IERC20(INFINITY_TOKEN).safeTransferFrom(msg.sender, address(this), amount);
    // emit event
    emit Staked(msg.sender, amount, duration);
}

Tools Used

Manual review

Recommended mitigation steps

Consider implementing staking "snapshots" where each and every staking event will have its own staking duration. Each staking snapshot can then be individually unstaked.

nneverlander commented 2 years ago

Will implement in a future upgrade

HardlyDifficult commented 2 years ago

This is a fair improvement consideration. Lowering risk and merging with the warden's QA report https://github.com/code-423n4/2022-06-infinity-findings/issues/345