Tadle

Summary

Native ETH to be withdrawn (via the transfer() method in the TokenManager#withdraw()) will be locked forever in the TokenManager contract, leading to loss of user (msg.sender)'s collateral (native ETH).

Vulnerability Details

When a user would like to withdraw their deposited-collateral (native ETH) from the CapitalPool contract, the user would call the TokenManager#withdraw().

Within the TokenManager#withdraw(), the following steps would be proceeded:

• 1/ WETH (wrappedNativeToken) would be transferred from the CapitalPool contract (capitalPoolAddr) to the TokenManager contract (address(this)).

• 2/ Then, the claimAbleAmount of the native ETH would be withdrawn from the WETH contract (IWrappedNativeToken(wrappedNativeToken)) to the TokenManager contract (address(this)).

• 3/ Finally, the claimAbleAmount of the native ETH-withdrawn would be transferred from the TokenManager contract (address(this)) to the user (msg.sender).
  https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L160-L166
  https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L168
  https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L169

However, within the TokenManager#withdraw(), the transfer() method of the native ETH would be used - when the the claimAbleAmount of the native ETH-withdrawn would be transferred from the TokenManager contract (address(this)) to a caller (msg.sender) during the step 3/ above:
(NOTE：This caller (msg.sender) would be an end user like an EOA or a smart contract wallet)
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L169

This is problematic because the transfer() method of the native ETH would only forward 2300 gas, which is not enough for the recipient to execute any non-trivial logic in a receive() or fallback(). Especially, it is not enough for the smart contract wallets like (Gnosis) Safes to receive funds, which require > 6k gas for the call to reach the implementation contract.

In this case (the step 3/ above), if a caller (msg.sender) would be a multisig or smart contract wallet like Safe that has the receive() requiring >2300 gas, their subsequent TokenManager#withdraw() call will fail permanently.
As a result, native ETH to be withdrawn (via the transfer() method in the TokenManager#withdraw()) will be locked forever in the TokenManager contract, leading to loss of user (msg.sender)'s collateral (native ETH).

(Reference: A similar finding in the past C4 contest is here)

Impact

• Native ETH to be withdrawn (via the transfer() method in the TokenManager#withdraw()) will be locked forever in the TokenManager contract, leading to loss of user (msg.sender)'s collateral (native ETH).

Tools Used

• Foundry

Recommendations

Within the TokenManager#withdraw(), consider using the low-level call method of the native ETH - instead of using the transfer() method of the native ETH like this: