Calling approve in capital pool with wrong arg causes withdrawals to always revert
Vulnerability Details 🔍 && Impact 📈
The approve() of CapitalPool.sol expects the token address (see here) to later call approve in that address. Yet TokenManager.sol in its _transfer() calls it like so: ICapitalPool(_capitalPoolAddr).approve(address(this));. See here.
Clearly address(this) is not a token address and trying to call approve() on it will fail as. This might have been a small oversight as exactly in the if statement on top of the approve call they are using the actual token address, see here.
Yet small oversight imapct is high and will happen in every call to _transfer() than involves from==CapitalPool, a.k.a. -> withdrawing funds out of the system. The current state of the system withdrawals is unusable as no matter what collateral you use the contracts will revert when trying to transfer from CapitalPool as the approval will always revert. Thus the funds inserted to the pool are stuck there.
Recommendations 🎯
The approve should be called passing the _token function parameter from _transfer().
Like so:
Lead Judging Commences
finding-TokenManager-approve-wrong-address-input
If we consider the correct permissioned implementation for the `approve()` function within `CapitalPool.sol`, this would be a critical severity issue, because the withdrawal of funds will be permanently blocked and must be rescued by the admin via the `Rescuable.sol` contract, given it will always revert [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/CapitalPool.sol#L36-L38) when attempting to call a non-existent function selector `approve` within the TokenManager contract. The argument up in the air is since the approval function `approve` was made permisionless, the `if` block within the internal `_transfer()` function will never be invoked if somebody beforehand calls approval for the TokenManager for the required token, so the transfer will infact not revert when a withdrawal is invoked. I will leave open for escalation discussions, but based on my first point, I believe high severity is appropriate.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.