LM_PC_Staking_v1&LM_PC_KPIRewarder - User can brick both contracts if he is first staker

[hats-bug-reporter[bot]]

Github username: -- Twitter username: @EgisSec Submission hash (on-chain): 0xd550ad6931bdc9f1efc508de69e199837236e00ad4e569aed416ef1d775fa619 Severity: medium

Description: Description\ The issue exists in both contracts and is technically the same, so I grouped them together.

Both LM_PC_Staking_v1 and LM_PC_KPIRewarder_v1 have a stake function.

function stake(uint amount)
        external
        virtual
        nonReentrant
        validAmount(amount)
        //@audit can stake before rewardsEnd and rewardRate is set
    {   
        address sender = _msgSender();

        _stake(sender, amount);

        // transfer funds to LM_PC_Staking_v1
        IERC20(stakingToken).safeTransferFrom(sender, address(this), amount);
    }

 function stake(uint amount)
        external
        override
        nonReentrant
        validAmount(amount)
    {
        if (stakingQueue.length >= MAX_QUEUE_LENGTH) {
            revert Module__LM_PC_KPIRewarder_v1__StakingQueueIsFull();
        }

        if (amount < minimumStake) {
            revert Module__LM_PC_KPIRewarder_v1__InvalidStakeAmount();
        }

        address sender = _msgSender();

        if (stakingQueueAmounts[sender] == 0) {
            // new stake for queue
            stakingQueue.push(sender);
        }
        stakingQueueAmounts[sender] += amount;
        totalQueuedFunds += amount;

        // transfer funds to LM_PC_Staking_v1
        IERC20(stakingToken).safeTransferFrom(sender, address(this), amount);

        emit StakeEnqueued(sender, amount);
    }

In both functions stake is of type uint = uint256. There are some tokens like cUSDCv3 that have a special case in their transfer logic.

function transferInternal(address operator, address src, address dst, address asset, uint amount) internal {
        if (isTransferPaused()) revert Paused();
        if (!hasPermission(src, operator)) revert Unauthorized();
        if (src == dst) revert NoSelfTransfer();

        if (asset == baseToken) {
            if (amount == type(uint256).max) {
                amount = balanceOf(src);
            }
            return transferBase(src, dst, amount);
        } else {
            return transferCollateral(src, dst, asset, safe128(amount));
        }
    }

If the amount specified is type(uint256).max then the entire balance of src will be transfered.

This is in issue in Inverter, as the above stake functions use the amount passed as is, meaning if type(uint256).max is set, then that's how much the code will credit to the user.

This allows for the first depositor in both contracts to completely brick the contracts, as in botch cases if another user stakes, a value will overflow, reverting the tx.

This is especially problematic if rewards are already sent to the contract, as it will make withdrawing them impossible. The owner will lose 100% of them.

The scenario isn't so uncommon as cUSDCv3 has a market cap of $62m, so it's not a niche token and the protocol states that it doesn't support fee-on transfer, rebsing or ERC777 tokens and cUSDCv3 is neither.

Similar issues: Codehawks Sherlock

Attack Scenario\

1. LM_PC_KPIRewarder_v1is deployed and the token is cUSDCv3.
2. The admin pre-sends the reward tokens to the contract.
3. Alice has dust amounts of cUSDv3 calls stake with amount = type(uint256).max.
4. totalQueuedFunds is now equal to type(uint256).max so anyone that calls stake will overflow the value and revert the tx.
5. There is no way to forcefully dequeue Alice's stake, so the funds are stuck.

The same scenario happens in LM_PC_Staking_v1, totalSupply will be set to type(uint256).max and no one else can stake after that and the reward tokens will again be stuck.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Change amount to type uint128.

[0xdeth]

Forgot to add in the report that the protocol is going to be deployed on Polygon and Linea as well. This attack will be even easier, as the attacker can front run the deployment of the contracts and stake.

[PlamenTSV]

I think this is too specific since only a singular token with weird behavior can create such impact. I believe an Orchestrator is 50/50 aware that it can cause issues, but I believe it's valid. I will give it both labels, as previous hats issues with singular weird tokens have been deemed low due to likelihood, but the impact here is high. Will leave the final severity to the sponsor.

[FHieser]

[FHieser]

I think this is highly unlikely but technically possible