Tranche.sol :: recoverERC20() If the target token is a Multiple Token Addresses all the fees can be stolen in form of target tokens.

Summary

recoverERC20() is utilized to retrieve all the fees. However, if the token target, where the fees are collected, consists of Multiple Token Addresses, all the fees can be stolen in form of target tokens.

Vulnerability Detail

recoverERC20() is used to recollect all the fees and send to the recipient.

function recoverERC20(address token, address recipient) external onlyManagement {
        if (token == address(_target)) revert ProtectedToken();
        uint256 balance = IERC20(token).balanceOf(address(this));
        IERC20(token).safeTransfer(recipient, balance);
    }

As you can observe, the function verifies if the token address is not the target to prevent sending the collected fees. However, the issue arises when the target is a Multiple Token Addresses (MTA). In this scenario, all the tokens can be stolen as this check is bypassed. MTA possesses the capability to transfer and manage balances using different addresses. If the target token is maliciously set as an MTA by the manager, it exposes all funds to potential theft.

Source: https://github.com/d-xo/weird-erc20?tab=readme-ov-file#multiple-token-addresses

This is a problem because the intended process involves sending the fees as target tokens to the adapter for burning, and subsequently sending them as underlying tokens to the feeRecipient, as indicated its description. However, through recoverERC20(), the manager can acquire the fees in the form of target tokens.

/// @notice Claim accumulated issuance fees. Redeem the fees in underlying.
/// @dev Only callable by management
/// @return Issuance fees in units of underlying token (e.g DAI) -> here
    function claimIssuanceFees() external onlyManagement returns (uint256) {
        uint256 fees = issuanceFees - 1; // Ensure that the slot is not cleared, for gas savings
        issuanceFees = 1;
        _target.safeTransfer(address(adapter), fees);
        (uint256 feesInUnderlying, ) = adapter.prefundedRedeem(feeRecipient);
        return feesInUnderlying;
    }

How you can see in the MockAdapter.sol the target tokens and burned and underlying tokens are sended to the feeRecipient.

 function prefundedRedeem(address to) public virtual returns (uint256 amountWithrawn, uint256 sharesRedeemed) {
        sharesRedeemed = MockERC20(target).balanceOf(address(this));
        // external call to `scale` is intentional so that `vm.mockCall` can work properly.
        amountWithrawn = (sharesRedeemed * this.scale()) / WAD;

        uint uBalance = MockERC20(underlying).balanceOf(address(lendingProtocol)); // balance of the external lending protocol
        require(uBalance >= amountWithrawn, "LendingProtocol: insufficient balance");

        SafeERC20.safeTransfer(MockERC20(target), address(lendingProtocol), sharesRedeemed);
        MockERC20(target).burn(address(lendingProtocol), sharesRedeemed); //-> burn target
        SafeERC20.safeTransferFrom(MockERC20(underlying), address(lendingProtocol), to, amountWithrawn); //-> send underlying
    }

As you can observe in the contest details, the admins are RESTRICTED, potentially causing this scenario.

Are the admins of the protocols your contracts integrate with (if any) TRUSTED or RESTRICTED?
RESTRICTED

Impact

All the fees can be stolen in form of target tokens by the manager.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/6313f34110b0d12677b389f0ecb3197038211e12/napier-v1/src/Tranche.sol#L597-L601 https://github.com/sherlock-audit/2024-01-napier/blob/6313f34110b0d12677b389f0ecb3197038211e12/napier-v1/src/Tranche.sol#L581-L587 https://github.com/sherlock-audit/2024-01-napier/blob/6313f34110b0d12677b389f0ecb3197038211e12/napier-v1/test/mocks/MockAdapter.sol#L58-L70

Tool used

Manual Review.

Recommendation

Restrict that the target token can`t be a Multiple Token Addresse token.

sherlock-audit / 2024-01-napier-judging

IvanFitro - Tranche.sol :: recoverERC20() If the target token is a Multiple Token Addresses all the fees can be stolen in form of target tokens. #17