DoS Of `TokenManager::Withdraw` due to typo in calling `CapitalPool::approve`
Summary
Vulnerability Details
Consider the call chain that occurs when a user calls TokenManager::withdraw to withdraw their funds.
User -> TokenManager::withdraw -> (if withdraw token == wrappedNativeToken) TokenManager::_transfer -> (if from == capitalPool && token.allowance(capitalPool, tokenManager) == 0) CapitalPool::approve(address(this)).
Now let's consider the execution of the CapitalPool::approve function, the main task of which is to give permission to spend tokenManager tokens.
Note that the parameter passed to the function is the address of the token, not the address to give allowance to.
Therefore, since approve is actually called with the tokenManager address, the call written below will be to the tokenManager address, not the output token. And tokenManager does not have a function with the required signature, so the call will be revert. So the whole call of the withdraw function will be revert.
Impact
On the one hand, this error blocks the basic functionality of the protocol and blocks user facilities.
On the other hand, the CapitalPool::approve function is external and anyone can give the desired allowance. Therefore, the functionality is blocked in case developers do not call this function themselves or users do not guess to call it themselves.
Thus, severity:medium
Tools Used
Manual review
Recommendations
Fix the typo
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.