User won't be able to get back their staked USDC/USDT token if their address is blacklisted in `LM_PC_Staking_v1.sol`

[hats-bug-reporter[bot]]

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xcc5288cac2a3745a964f41e886a1bf16b3c7238ee2fde72e52333d7bd5130de4 Severity: medium

Description: Description\ LM_PC_Staking_v1.sol contract provides a mechanism for users to their tokens to earn rewards. This is done by calling LM_PC_Staking_v1.stake() function. When the users wants to get back their staked tokens with rewards, LM_PC_Staking_v1.unstake()can be called by users.

Per the discussion with protocol team, its understood that LM_PC_Staking_v1 admin is free to consider any token as stakingToken except FOT/rebasing/callback tokens.

This issue is specifically for tokens like USDC which has a blacklist() function and it is used to blacklist any address by USDC admin. This can be checked here

The contracts will be deployed on Optimism,Polygon, linea.

Consider below scenario to understand the issue better:

1) LM_PC_Staking_v1 is deployed on Optimism and admin has considered USDC as `stakingToken.

2) Alice wants to stake her USDC so she calls LM_PC_Staking_v1.stake() and transfer the USDC to LM_PC_Staking_v1 contract.

170        IERC20(stakingToken).safeTransferFrom(sender, address(this), amount);

3) After few days/months, Alice decides to unstake the staked tokens i.e to get back USDC from LM_PC_Staking_v1 so she calls LM_PC_Staking_v1.unstake(). Alice finds that her address is blacklisted by USDC.

4) When LM_PC_Staking_v1 contract ties to transfer the USDC to Alice address:

    function unstake(uint amount)
        external
        virtual
        nonReentrant
        validAmount(amount)
    {
@>      address sender = _msgSender();
        // Update rewardValue, updatedTimestamp and earned values
        _update(sender);

        // Reduce balances accordingly
        _balances[sender] -= amount;
        // Total supply too
        totalSupply -= amount;

        // Transfer funds back to sender
@>      IERC20(stakingToken).safeTransfer(sender, amount);

The transaction fails as the transfer function checks the recipient address is blacklisted or not.

Below is the transfer method of USDC,

    function transfer(address to, uint256 value)
        external
        override
        whenNotPaused
        notBlacklisted(msg.sender)
        notBlacklisted(to)                   @audit// checks recipient is blacklisted or not
        returns (bool)
    {
        _transfer(msg.sender, to, value);
        return true;
    }

USDC notBlacklisted() modifier,

    modifier notBlacklisted(address _account) {
        require(
            !blacklisted[_account],
            "Blacklistable: account is blacklisted"
        );
        _;
    }

So the USDC transfer in our case will awalys revert and Alice wont be able to get back her staked USDC.

Impact\ Users wont be able to get back their deposited stakingToken i.e USDC/USDT if their address is blacklisted as LM_PC_Staking_v1 can not transfer USDC to blacklisted address.

Recommendation to fix\ Recommend to allow the recipient address as function param in LM_PC_Staking_v1.unstake() function.

For example:

-    function unstake(uint amount)
+    function unstake(address recipient, uint amount)
        external
        virtual
        nonReentrant
        validAmount(amount)
    {
+     require(recipient != address(0), "Zero address");
+     require(recipient != address(this), "can not transfer to staking contract");
        address sender = _msgSender();
        // Update rewardValue, updatedTimestamp and earned values
        _update(sender);

        // Reduce balances accordingly
        _balances[sender] -= amount;
        // Total supply too
        totalSupply -= amount;

-        // Transfer funds back to sender
-        IERC20(stakingToken).safeTransfer(sender, amount);
+        // Transfer funds back to recipient address
+        IERC20(stakingToken).safeTransfer(recipient, amount);

        // If the user has earned something
        if (rewards[sender] != 0) {
            // distribute rewards
            _distributeRewards(sender);
        }

        emit Unstaked(sender, amount);
    }

Note: If the above change is consider then reward recipient should also be changed in _distributeRewards()

[NicolaMirchev]

Hey there,

The following should be considered low at most because:

• It would be only user loss, if he gets blacklisted (he doesn't harm the protocol)
• The main idea behind blocklisting address is to "freeze" the funds, which are in user's wallet (he cannot transfer them out). So at worst (which probability is very low) the victim will suffer the consequences of being "blocklisted" as if the funds were in his wallet. As soon as he is removed from the blocklist, he will be able to unstake.

[0xRizwan]

This issue is of Medium severity as originally submitted and judged. The end user is affected here due to his address being blacklisted. This make me him impossible to unstake from Inverter contracts. Sending the stakes tokens to users own address i.e msg.sender is actually a limitation and could act as barrier here due to blacklisting issue. Practically, this issue would also have adverse impact on Inverter staking contract as unstake would fail in above issue. Recommendation is simple to follow i.e allowing recipient address to get back the staked tokens, the recipient address could be msg.sender himself if he is not blacklisted or some of his desired address. I believe, Medium severity is justified here.

[FHieser]

I know where you are coming from but we currently dont want to get into the topic of circumventing blacklist restrictions, as we dont know the size of the topic yet

[0xmahdirostami]

@NicolaMirchev @FHieser thank you, I think low label is good for this issue.

As the following:

• It would be only user loss, if he gets blacklisted (he doesn't harm the protocol)
• The main idea behind blocklisting address is to "freeze" the funds, which are in user's wallet (he cannot transfer them out). So at worst (which probability is very low) the victim will suffer the consequences of being "blocklisted" as if the funds were in his wallet. As soon as he is removed from the blocklist, he will be able to unstake.
• just like sell and sell for it could mitigate.

[0xRizwan]

Issue impact is high and severity is confirmed to be Medium. Will wait for @FHieser, based on our last discussion on this topic.

[FHieser]

We acknowledge this issue as Medium severity. In addition to this issue in audit report, we will add the disclaimer for users in case of USDC/USDT risks.