Tadle

Relevant Links

• https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L169

• https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/utils/Rescuable.sol#L70

Summary

The application uses payable(address).transfer for ETH transfers. Perhaps because we wanted to avoid creating reentrancy vulnerabilities by calling into the contract payable(address).call{value: value_here}(""). This however, makes the application incompatible with smart contract wallets which might have some code run in their receive function. For example Gnosis Safe.

Impact

We can't withdraw ( TokenManager.withdraw ) or rescue ( Rescuable.rescue ) funds to smart contract wallets.

Tools Used

Manual review

Recommendations

1. rather use payable(address).call{value: amount_to_transfer}("") for eth transfer and add the necessary checks-effects-interractions pattern and modifier to prevent reentrancy vulnerabilities.

2. To completely eliminate the possibility of any reentrancy vulnerability through the transfer of eth, switch all the withdraw methods that implement payable(address).transfer to instead withdraw the wrapped native token which is an ERC20 token. For example WETH in the case of ethereum or wrapped matic in the Polygon. Switch all the methods that act as a currency ( Eth in the case of ethereum or matic in the case of polygon) deposit methods to receive the currency and convert it to the wrapped asset version before effectively depositing it with the appropriate accounting in the application. Highly recommended.