Core Contracts

Summary

The Treasury contract directly calls IERC20(token).transferFrom for deposits and IERC20(token).transfer for withdrawals without handling potential return values. This is problematic because not all ERC20 tokens strictly adhere to the standard. Some tokens, like USDT (Tether), do not return a boolean value upon transfer, and instead, they revert on failure. This can lead to unexpected behavior where funds do not move as expected, but the contract still updates its internal balances, creating inconsistencies between recorded and actual balances.

Vulnerability Details

In the deposit function, the contract assumes that transferFrom will always succeed. If the token being deposited is non-standard (like USDT) and does not return a boolean, the call will not revert, but the function will proceed even if no tokens were actually transferred. Furthermore, the _balances[token] and _totalValue variables will still be updated regardless of whether tokens were received, leading to phantom balances in the contract.

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/Treasury.sol#L50-L52

Again, in the withdraw function, the function does not check whether IERC20(token).transfer(recipient, amount); actually succeeds. If the token does not return a boolean (like USDT), it could revert instead, meaning that while no tokens are actually transferred, the contract still updates _balances[token] and _totalValue.

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/Treasury.sol#L73-L75

Impact

If a non-standard token like USDT is deposited, it could fail to transfer properly but still be recorded as deposited. This can lead to users believing their funds are stored in the contract when they are not. Furthermore, the treasury could report holding more tokens than it actually does. This could lead to incorrect allocations, miscalculations, or failed withdrawals when attempting to distribute funds that do not exist. Finally, if a manager attempts to withdraw funds from a token that reverts on transfer, the balance will be reduced even though the transaction failed, resulting in a permanent loss of access to those funds.

Tools Used

• Manual code review

Recommendations

To prevent this issue, the contract should use SafeERC20 from OpenZeppelin, which properly handles non-standard token behavior.