Withdraw/Redeem functions of GaugeUpgradeable can fail due to blocked USDT/USDC accounts

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0x883a93c8b124c4fa01686ea659bb38a27ca16f395d82c2ee6e5053341ddd624b
Severity: medium

Description:
Title
Withdraw/Redeem functions can fail due to blocked USDT/USDC accounts

Description
Withdraw and redeem functions in smart contracts can fail if they involve transferring USDT, USDC, or other centralized stablecoins from blocked or blacklisted accounts. Both Tether (USDT) and USD Coin (USDC) are issued by centralized entities with the authority to freeze or blacklist addresses. When an account is blacklisted, any attempts to transfer these tokens from the blacklisted address will be blocked, causing the transaction to fail.

This issue can significantly impact decentralized finance (DeFi) protocols, which rely on seamless token transfers for operations like withdrawals and redemptions. If a user's address or the contract itself gets blacklisted, users may be unable to withdraw their funds, leading to a loss of trust and potentially severe financial consequences.

A potential way to fix this issue is to allow users to specify an alternative address for the transfer to go to. However, the contract must still validate that the withdrawal amount is correctly debited from the msg.sender's balance to prevent any unauthorized transfers. This ensures that users cannot steal other users' assets by specifying different addresses.

Attack Scenario

The withdraw functions listed in the PoC section of this submission show that users can initiate a energency withdraw of the TOKEN state variable which is the underlying lp token of the GaugeUpgradeable contract. This state var is set during initialization and can be set to any token, including either USDC or USDT, in fact as TOKEN is a lp token, it is exceptionally likely that it will be USDC/USDT.

Both USDC and USDT have a blacklist functionality, thus when transfer() is called in either of these token contracts, the 'from' and 'to' address are checked againt, below you can see the blacklist modifier of USDC

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

Therefor is a user interacts with Fenix Finance and deposits USDC or USDT in a GaugeUpgradeable contract instance with USDC/USDT as the 'TOKEN' state var and their USC/USDT address gets blacklisted, they will be unable to withdraw their assets.

This is because although the internal accounting checks _balances[msg.sender] which is good, the safeTransfer call has the 'to' address also as msg.sender, the user has no way to select what address to send their assets to so when the call goes to the USDC/USDT transfer call
msg.sender will be checked against the blacklist. This can thus prevent users from withdrawing their funds in such instances.

Attachments

1. Proof of Concept (PoC) File

['289']

289:     function emergencyWithdraw() external nonReentrant {
290:         require(emergency, "emergency");
291:         require(_balances[msg.sender] > 0, "no balances");
292:
293:         uint256 _amount = _balances[msg.sender];
294:         _totalSupply = _totalSupply - (_amount);
295:         _balances[msg.sender] = 0;
296:
297:         IERC20(TOKEN).safeTransfer(msg.sender, _amount); // <= FOUND
298:         emit Withdraw(msg.sender, _amount);
299:     }

['301']

301:     function emergencyWithdrawAmount(uint256 _amount) external nonReentrant {
302:         require(emergency, "emergency");
303:         require(_balances[msg.sender] >= _amount, "no balances");
304:
305:         _totalSupply = _totalSupply - (_amount);
306:         _balances[msg.sender] -= _amount;
307:         IERC20(TOKEN).safeTransfer(msg.sender, _amount); // <= FOUND
308:         emit Withdraw(msg.sender, _amount);
309:     }

2. Revised Code File (Optional)

Function Block 1

--- function emergencyWithdraw() external nonReentrant {
+++ function emergencyWithdraw(address to) external nonReentrant 
        require(emergency, "emergency");
        require(_balances[msg.sender] > 0, "no balances");
+++     require(to != address(0), "address(0)");
+++     require(to != address(this), "not a withdraw");

        uint256 _amount = _balances[msg.sender];
        _totalSupply = _totalSupply - (_amount);
        _balances[msg.sender] = 0;

---     IERC20(TOKEN).safeTransfer(msg.sender, _amount);
+++     IERC20(TOKEN).safeTransfer(to, _amount);
        emit Withdraw(msg.sender, _amount);
    }

Function Block 2

--- function emergencyWithdrawAmount(uint256 _amount) external nonReentrant {
+++ function emergencyWithdrawAmount(address to, uint256 _amount) external nonReentrant {
        require(emergency, "emergency");
        require(_balances[msg.sender] >= _amount, "no balances");
+++     require(to != address(0), "address(0)");
+++     require(to != address(this), "not a withdraw");

        _totalSupply = _totalSupply - (_amount);
        _balances[msg.sender] -= _amount;
---     IERC20(TOKEN).safeTransfer(msg.sender, _amount);
+++     IERC20(TOKEN).safeTransfer(to, _amount);
        emit Withdraw(msg.sender, _amount);
    }

The solutions above allow the user to specify what address to withdraw to without needing to modify any internal accounting
operations.

[0xmahdirostami]

Out of scope