#41456 [SC-Critical] `executeRewardDistributionYeet` will count user withdraws as rewards

Submitted on Mar 15th 2025 at 13:46:21 UTC by @Pyro for Audit Comp | Yeet

  • Report ID: #41456

  • Report Type: Smart Contract

  • Report severity: Critical

  • Target: https://github.com/immunefi-team/audit-comp-yeet/blob/main/src/StakeV2.sol

  • Impacts:

    • Protocol insolvency

    • Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

    • Smart contract unable to operate due to lack of token funds

Description

Brief/Intro

executeRewardDistributionYeet will count user withdraws as rewards due to the way we calculate the rewards inside accumulatedDeptRewardsYeet

Vulnerability Details

accumulatedDeptRewardsYeet calculates the rewards generated by removing totalSupply from stakingToken.balanceOf(address(this))

    function accumulatedDeptRewardsYeet() public view returns (uint256) {
        return stakingToken.balanceOf(address(this)) - totalSupply;
    }

Where that difference is used inside executeRewardDistributionYeet as generated rewards, which are swapped for vault shares.

    function executeRewardDistributionYeet( ... ) external onlyManager nonReentrant {
    
        uint256 accRevToken0 = accumulatedDeptRewardsYeet();
        require(accRevToken0 > 0, "No rewards to distribute");
        require(swap.inputAmount <= accRevToken0, "Insufficient rewards to distribute");

        stakingToken.approve(address(zapper), accRevToken0);
        IERC20 token0 = IKodiakVaultV1(stakingParams.kodiakVault).token0();
        IERC20 token1 = IKodiakVaultV1(stakingParams.kodiakVault).token1();

        uint256 vaultSharesMinted;
        require(
            address(token0) == address(stakingToken) || address(token1) == address(stakingToken),
            "Neither token0 nor token1 match staking token"
        );

However accumulatedDeptRewardsYeet forgets to consider the fact that startUnstake keeps the staking tokens inside the contract, but lowers totalSupply by unStakeAmount as it sets that amount for a vest.

    function startUnstake(uint256 unStakeAmount) external {
        // ...
        balanceOf[msg.sender] -= unStakeAmount;
        totalSupply -= unStakeAmount;

        // ...
    }

In short all user withdraws will be counted by the contract as rewards, meaning that if swapped these withdraws will not be refundable, or even worse - they would be taken from the users that didn't appoint a withdraw, making the contract insolvent.

Impact Details

Contract is insolvent due to it counting all pending withdraws as rewards and swapping them in a different token.

References

none are needed

Proof of Concept

Proof of Concept

  1. 10 users deposit, each with 100 tokens

  2. 1 schedules a withdraw for 100 tokens

  3. Admin distributes rewards (these 100 tokens are counted towards the rewards)

  4. User withdraws his 100 tokens

step 3 and 4 both costed 100 tokens, meaning the contract has 800 actual tokens and 900 deposited balances, making it insolvent.

Was this helpful?