StraawHaat - A malicious user can manipulate the pool using a weird token similar to `cUSDCv3`

[sherlock-admin3]

StraawHaat

High

A malicious user can manipulate the pool using a weird token similar to cUSDCv3

Summary

A malicious user can manipulate the pool using a weird token similar to cUSDCv3.

Some tokens (e.g., cUSDCv3) contain a special case for amount == type(uint256).max in their transfer functions that results in only the user's balance being transferred.

This may cause issues with systems that transfer a user-supplied amount to their contract and then credit the user with the same value in storage (e.g., Vault-type systems) without checking the amount that has actually been transferred.

Vulnerability Detail

The protocol will use any type of tokens:

If you are integrating tokens, are you allowing only whitelisted tokens to work with the codebase or any complying with the standard?

• Any type of ERC20 token. Pools are permissionless. So users can open pools even with weird tokens. Issues regarding any weird token will be valid if they have Med/High impact.

A user can open a pool (or use an open pool that has a similar token) and manipulate the balance.

A user can open a pool(or use an open pool that has a similar token as cUSDCv3) and then add to the position by specifying amountToAdd = type(uint256).max to manipulate the balance:

    function addToPosition(uint256 tokenId, uint256 amountToAdd) external override nonReentrant {
        _requireOnlyOperatorOrOwnerOf(tokenId);
        require(amountToAdd > 0, "0 amount"); // addToPosition: amount cannot be null

        _updatePool();
        address nftOwner = ERC721Upgradeable.ownerOf(tokenId);
        _harvestPosition(tokenId, nftOwner);

        StakingPosition storage position = _stakingPositions[tokenId];

        // we calculate the avg lock time:
        // lock_duration = (remainin_lock_time * staked_amount + amount_to_add * inital_lock_duration) / (staked_amount + amount_to_add)
        uint256 remainingLockTime = _remainingLockTime(position);
        uint256 avgDuration = (remainingLockTime * position.amount + amountToAdd * position.initialLockDuration)
            / (position.amount + amountToAdd);

        position.startLockTime = _currentBlockTimestamp();
        position.lockDuration = avgDuration;

        // lock multiplier stays the same
        position.lockMultiplier = getMultiplierByLockDuration(position.initialLockDuration);

        // handle tokens with transfer tax
        amountToAdd = _transferSupportingFeeOnTransfer(stakedToken, msg.sender, amountToAdd);

        // update position
        position.amount = position.amount + amountToAdd;
        _stakedSupply = _stakedSupply + amountToAdd;
        _updateBoostMultiplierInfoAndRewardDebt(position);

        emit AddToPosition(tokenId, msg.sender, amountToAdd);
    }

If user specifying amountToAdd = type(uint256).max, It will update his state by a significant amount but send much fewer tokens. This can cause serious discrepancies with the protocols intended logic.

The same attack is possible also in MasterchefV2.sol in deposit() function:

    function deposit(uint256 pid, uint256 amount) external override {
        _modify(pid, msg.sender, amount.toInt256(), false); 

        if (amount > 0) _farms[pid].token.safeTransferFrom(msg.sender, address(this), amount);
    }

This vulnerability is possible everywhere in the protocol where it is possible to use this type of token.

Impact

The use of this type of tokens will lead to manipulation in different parts of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L397-L428 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L295-L299

Tool used

Manual Review

Recommendation

It is best not to use such type of tokens.

Duplicate of #545