Core Contracts

Summary

The Treasury contract uses the IERC20.transferFrom function to transfer tokens from the user to the contract. However, the return value of this function is not checked, which can lead to improper accounting and potential loss of funds if the transfer fails silently.

Vulnerability Details

Source

• Line 75

• Lines 50-52

The Treasury::deposit and Treasury::withdraw functions in the Treasury contract use the IERC20.transferFrom and IERC20.transfer functions, respectively, without checking their return values. This can result in silent failures, leading to incorrect token balances and potential loss of funds.

Proof of Concept

Example Scenario

1. Deposit Function:

• A user calls the Treasury::deposit function to deposit tokens into the treasury.

• The contract calls IERC20(token).transferFrom(msg.sender, address(this), amount).

• If the transfer fails silently (returns false), the contract still updates the _balances and _totalValue variables, leading to incorrect accounting.

2. Withdraw Function:

• A manager calls the Treasury::withdraw function to withdraw tokens from the treasury.

• The contract calls IERC20(token).transfer(recipient, amount).

• If the transfer fails silently (returns false), the contract still updates the _balances and _totalValue variables, leading to incorrect accounting and potential loss of funds.

Example Scenario

1. Silent Transfer Failure:

• The user calls the Treasury::deposit function to deposit tokens into the treasury.

• The transfer fails silently, but the contract updates the _balances and _totalValue variables.

• The user can then withdraw more tokens than they actually deposited, leading to a loss of funds for the treasury.

2. Improper Accounting:

• A manager calls the Treasury::withdraw function to withdraw tokens from the treasury.

• The transfer fails silently, but the contract updates the _balances and _totalValue variables.

• The user ends up with fewer tokens than expected, leading to a loss of funds for the user.

Impact

The improper handling of ERC20 transfers can lead to:

• Incorrect balance tracking.

• Potential loss of funds for both the user and the treasury.

• Exploits where users withdraw more tokens than they deposited.

Tools Used

• Manual code review

• Foundry

Recommendations

The return value of the transferFrom and transfer calls should be checked to ensure that the transfers are successful. This can be done using the SafeERC20 library from OpenZeppelin, which handles the return value checks and reverts the transaction if the transfer fails.

Mitigation Steps

In Treasury::deposit:

In Treasury::withdraw: