In the StakeV2 contract, when the rageQuit and unstake functions are used, the order of the user's vestings array changes. This can lead to calling functions with incorrect indexes, causing users to perform unintended unstake or rageQuit operations, which may result in fund loss.
Vulnerability Details
This issue arises due to the way the vestings array is managed. When a user calls startUnstake, the request is stored at a specific index in the array. However, when unstake(index) or rageQuit(index) is executed, the array is reordered, causing indexes to shift. If the user or dApp performs an operation based on previous indices, or if the dApp does not update the page, they may unintentionally unstake the wrong position using unstake(index) or rageQuit(index)
Example Scenario
The user initiates 5 different startUnstake requests:
Unstake array: (a, b, c, d, e)
Assigned indexes: (0, 1, 2, 3, 4)
The user calls unstake(2), removing c. The array is updated as follows:
New array: (a, b, d, e)
New indexes: (0, 1, 2, 3)
Important:d's previous index 3 is now 2, and e's previous index 4 is now 3.
If the user or a dApp, based on outdated information, calls rageQuit(3), expecting to exit d, they would actually exit e instead.
Impact Details
This vulnerability can cause users to exit the wrong vesting position, leading to unintended rageQuit operations and potential fund loss.
A bool isUnstake variable can be added to the Vesting struct. Additionally, minimum stake and minimum unstake conditions can be introduced to protect against DOS attacks.
/// @notice The struct used to store the vesting information
struct Vesting {
uint256 amount;
uint256 start;
uint256 end;
+ bool isUnstake;
}
- function _remove(address addr, uint256 _index) private {
- Vesting[] storage arr = vestings[addr];
- require(_index < arr.length, "index out of bound");
- uint256 length = arr.length;
- for (uint256 i = _index; i < length - 1; i++) {
- arr[i] = arr[i + 1];
- }
- arr.pop();
- }
function unstake(uint256 index) external {
require(block.timestamp >= vestings[msg.sender][index].end, "Vesting period has not ended");
+ require(!vestings[msg.sender][index].isUnstake, "Unstaked");
_unstake(index);
}
function rageQuit(uint256 index) external {
+ require(!vestings[msg.sender][index].isUnstake, "Unstaked");
_unstake(index);
}
function _unstake(uint256 index) private {
Vesting memory vesting = vestings[msg.sender][index];
(uint256 unlockedAmount, uint256 lockedAmount) = calculateVesting(vesting);
require(unlockedAmount != 0, "No unlocked amount");
stakingToken.transfer(msg.sender, unlockedAmount);
stakingToken.transfer(address(0x000000dead), lockedAmount);
- _remove(msg.sender, index);
+ vestings[msg.sender][index].isUnstake=true;
if (lockedAmount > 0) {
emit RageQuit(msg.sender, unlockedAmount, lockedAmount, index);
} else {
emit Unstake(msg.sender, unlockedAmount, index);
}
stakedTimes[msg.sender]--;
}
