absence of Validation of Fee Receiver Address Leading to Loss of Funds

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/ousgInstantManager.sol#L650-L656 https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/ousgInstantManager.sol#L315-L318 https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/ousgInstantManager.sol#L450-L452

Vulnerability details

Description

the vulnerability here is arises from the lack of validation checks on the feeReceiver address when it is set or modified. The contract does not ensure that the feeReceiver is capable of managing or forwarding the received USDC. This oversight allows the feeReceiver to be set to the contract address itself or another contract address that does not expect to receive USDC, leading to potential fund lock-up. here the mint() and redeem() functions, where fees are transferred to the feeReceiver. The setting of the feeReceiver occurs without checks in the setFeeReceiver() function

function setFeeReceiver(address _feeReceiver) external override onlyRole(DEFAULT_ADMIN_ROLE) {
    require(_feeReceiver != address(0), "FeeReceiver cannot be 0x0");
    feeReceiver = _feeReceiver;
}

and here

// Inside _mint() function
usdc.transferFrom(msg.sender, feeReceiver, usdcfees);

and here

// Inside _redeem() function
usdc.transfer(feeReceiver, usdcFees);

Impact

if the feeReceiver is incorrectly set to a non-custodial address or the contract itself, the collected fees become irretrievable within the contract's logic, effectively locking the funds. This can result in a significant loss over time, especially as the contract continues to collect fees from minting and redeeming operations.

Proof of Concept

Let's assume that the contract has collected 10,000 USDC in fees over various minting and redeeming operations, directed to what is believed to be a valid fee management address.

• The contract admin inadvertently sets the feeReceiver to the contract address itself, perhaps misunderstanding its role or due to a clerical error. Configuration: feeReceiver Address: 0xContractAddress (the contract's own address) Collected Fees: 10,000 USDC
• The 10,000 USDC in fees are sent to the contract's address. Without a mechanism in place for the contract to withdraw or distribute these USDC tokens, they become locked within the contract.
• Attempts to correct the feeReceiver address after fees have been transferred are futile regarding the already transferred funds, as there's no function within the contract to "rescue" or redirect these tokens.

  Tools Used

  manual review

  Recommended Mitigation Steps

  it's need to ensure the new feeReceiver address is not the contract itself or another contract incapable of handling USDC

Assessed type

Other

[c4-pre-sort]

0xRobocop marked the issue as insufficient quality report

[c4-sponsor]

cameronclifton (sponsor) disputed

[cameronclifton]

This is an explicit design decision. It is not out of the realm of possibilities that the fee receiver can be set to the contract and then the USDC can later be withdrawn via the retrieveToken function. Permissioned admin functions calls are assumed to be correct and not malicious.

[3docSec]

Admin mistake -> OOS

[c4-judge]

3docSec marked the issue as unsatisfactory: Out of scope