Incorrect argument given to `CapitalPool:approve` in `TokenManager:_transfer` may lead to `tillIn` and `withdraw` functions reverting.
Summary
The tillIn and withdraw functions in TokenManager.sol will malfunction because an incorrect argument given to the approve function of CapitalPool.sol in the _transfer function.
Vulnerability Details
TokenManager.sol has an interanl _transfer function used to transfer tokens to and from the CapitalPool.sol contract. If tokens are transfered from the CapitalPool and the allowance of TokenManager.sol is 0 then TokenManager.sol calls CapitalPool:approve to give max allowance to TokenManager.sol to transfer any amount of _token from CapitalPool.
CapitalPool:approve takes the _token's address that is approved to be used by TokenManager as an argument. When the function is called in TokenManager, address(this) is given as the argument instead of the token address to call approve. This causes the _transfer function in TokenManager to revert if the contract does not have approval for the token beforehand.
Impact
TokenManager:tillIn and TokenManager:withdraw functions will not be callable until the CapitalPool:approve function is called directly.
Tools Used
Recommendations
Replace address(this) with _token on TokenManager.sol#L247.
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.