Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
The function stakeV2::accumulatedDeptRewardsYeet is intended to return the dust amount collected from swaps, allowing the manager to convert it into the reward token. However, the logic determining the available amount of Yeet tokens for conversion is incorrect, potentially leading to miscalculations.
Vulnerability Details
The comment on top of the function accumulatedDeptRewardsYeet states that:
/// @notice The function used to calculate the accumulated rewards that gets return by the zapper since swaps are not 100% efficient
// @dev The function is used to calculate the rewards that are not distributed yet
/audit-comp-yeet/src/StakeV2.sol:148
148: function accumulatedDeptRewardsYeet() public view returns (uint256) {
149: return stakingToken.balanceOf(address(this)) - totalSupply;
150: }
This function get called inside executeRewardDistributionYeet to check that no staking token would be allowed to got converted into reward tokens.
The call to the function executeRewardDistributionYeet is trusted so there would no attacker involved or lose of assets in normal flow , but this function accumulatedDeptRewardsYeet will always return wrong value because when user calls startUnstake function we subtract his staked tokens from totalSupply.
/audit-comp-yeet/src/StakeV2.sol:247
247: function startUnstake(uint256 unStakeAmount) external {
248: require(unStakeAmount > 0, "Amount must be greater than 0");
249: require(stakedTimes[msg.sender] < STAKING_LIMIT, "Amount must be less then the STAKING_LIMIT constant"); // DOS protection https://github.com/Enigma-Dark/Yeet/issues/12
250: _updateRewards(msg.sender);
251: uint256 amount = balanceOf[msg.sender];
252: require(amount >= unStakeAmount, "Insufficient balance");
253:
254: balanceOf[msg.sender] -= unStakeAmount;
255: totalSupply -= unStakeAmount;
256:
257: uint256 start = block.timestamp;
258: uint256 end = start + VESTING_PERIOD;
259: vestings[msg.sender].push(Vesting(unStakeAmount, start, end));
260: stakedTimes[msg.sender]++;
261: emit VestingStarted(msg.sender, unStakeAmount, vestings[msg.sender].length - 1);
262: }
As it can be seen from the above code at line 254 and 255 we subtract the amount being unstacked from user balance and total supply. that why the function accumulatedDeptRewardsYeet return values will also include the amount waiting for vesting peroid to pass and get withdraw from stakeV2 contract.
Impact Details
The impact in normal case is only limited to incorrect return value but if manager assumes this as correct value and provide this value as swap.inputAmount in call to executeRewardDistributionYeet than this issue will lead to lose of assets for protocol or end users.
References
Proof of Concept
Add following test case to stakeV2.test.sol::StakeV2_HandleExcessDebt and run with command : forge test --mt test_accumulatedDeptRewardsYeet_is_not_correct -vvv
function test_accumulatedDeptRewardsYeet_is_not_correct() public {
address owner = address(this);
address manager = address(this);
KodiakVaultV1 kodiakVault = new KodiakVaultV1(token, wbera);
SimpleZapperMock mockZapper = new SimpleZapperMock(kodiakVault.token0(), kodiakVault.token1());
StakeV2 stakeV2 = new StakeV2(token, mockZapper, owner, manager, IWETH(wbera));
token.mint(address(this), 100 ether);
token.approve(address(stakeV2), 50 ether);
stakeV2.stake(50 ether);
stakeV2.startUnstake(40 ether);
// simulate debt by adding excess token0
token.transfer(address(stakeV2), 50 ether);
assertEq(90 ether, stakeV2.accumulatedDeptRewardsYeet()); // Audit : this should have return 0
//zapper
mockZapper.setReturnValues(1, 1); // does not matter
stakeV2.depositReward{
value: 1 ether
}();
assertEq(100 ether, token.balanceOf(address(stakeV2)));
stakeV2.executeRewardDistributionYeet(
IZapper.SingleTokenSwap(stakeV2.accumulatedDeptRewardsYeet(), 0, 0, address(0), ""),
IZapper.KodiakVaultStakingParams(address(kodiakVault), 0, 0, 0, 0, 0, address(0)),
IZapper.VaultDepositParams(address(0), address(0), 0)
);
assertEq(10 ether, token.balanceOf(address(stakeV2))); // now stakeV2 only hold 10 tokens
assertEq(90 ether, token.balanceOf(address(mockZapper))); // all the token are now in mockZapper
vm.warp(block.timestamp + 11 days);
vm.expectRevert();
stakeV2.unstake(0);
}
