Tadle

Summary

The Rescuable contract includes safe_transfer and safe_transfer_from functions to handle token transfers. These functions are used by the TokenManager contract for depositing and withdrawing funds. However, the current implementation is vulnerable to issues with ERC20 tokens that do not revert on failure but instead return false. This behavior is technically compliant with the ERC20 standard but can lead to unexpected issues if not handled correctly.

Vulnerability Details

Some ERC20 tokens do not revert on failure; instead, they return false when the transfer or transferFrom fails (e.g., ZRX, EURS). The current implementation of _safe_transfer and _safe_transfer_from in the Rescuable contract does not account for this case. It only checks if the low-level call to the token contract was successful, but it does not check the return value of the token's function.

Current Implementation:

• Potential Token Transfers with Insufficient Funds: If an ERC20 token function returns false but does not revert, the _safe_transfer and _safe_transfer_from functions will not catch this scenario. This means that the functions might execute as if the transfer was successful, even if it was not.

• User Exploitation: Attackers could exploit this behavior to "transfer" tokens they do not actually own or create fake offers and withdraw amounts that were not properly transferred, leading to potential theft and loss of funds.

PoC

You can observe the aforementioned behavior with the following Proof of Concept:

Steps:

1. Create a new file called CapitalPool.t.sol in the test folder.

2. Copy and paste the following content into it:

1. Run the test using the command: forge test --mt test_CallWillSucceed --via-ir -vv

Console output

As demonstrated, the test passes because the low-level call was successful, and we do not check for the returned value from the function call. This mirrors the issue found in the safe_transfer and safe_transfer_from functions.

Impact

Attackers could exploit this vulnerability to drain funds from the protocol by creating offers and withdrawing amounts that were not actually transferred.

Tools Used

VSCode, Foundry

Recommendations

Update the _safe_transfer and _safe_transfer_from functions to use OpenZeppelin’s safeTransfer and safeTransferFrom methods. This ensures that both the success of the transaction and the return value are properly handled.