Core Contracts

Summary

The Treasury contract manages protocol funds with role-based access control, supporting deposits, withdrawals, and fund allocation. However, there is a security issue in the token transfer functions that could lead to failures or vulnerabilities due to improper token transfer methods.

Issue Details

1. Missing Safe Transfer Mechanism

The contract uses IERC20(token).transferFrom and IERC20(token).transfer for handling token transfers. However, these functions do not handle cases where tokens do not return a boolean value upon failure, which can cause silent failures in certain ERC-20 implementations.

Vulnerable Code in Deposit Function

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

• If transferFrom fails without reverting, the transaction will proceed, leading to incorrect balance updates.

• Using SafeERC20.safeTransferFrom would ensure safe token transfers.

Vulnerable Code in Withdraw Function

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

• Direct use of transfer does not guarantee success for non-standard ERC-20 tokens.

• Implementing SafeERC20.safeTransfer would prevent silent failures.

Impact

1. Potential Fund Loss: Users may assume funds are successfully transferred while tokens remain locked in the contract.

2. Silent Failures: Transactions could complete without actually transferring funds.

3. Incompatibility with Some ERC-20 Tokens: Some tokens do not adhere to the expected return values, leading to unexpected behavior.

Recommendation

Replace direct token transfers with OpenZeppelin’s SafeERC20 library to ensure robust and secure token transfers.