Passing the wrong parameter to the CapitalPool::approve function causes token transfer failure
Summary
A vulnerability was identified in the TokenManager::_transfer function, where the logic for handling token approvals from the CapitalPool contract is flawed. This results in the transfer operation being reverted, effectively blocking the transfer of tokens from CapitalPool to the intended recipient.
Vulnerability Details
In the TokenManager::_transfer function, there is logic to check if the _from address is the CapitalPool contract address, and if the token allowance of CapitalPool to TokenManager is equal to 0. If so, it calls the CapitalPool::approve function to approve tokens from CapitalPool for TokenManager. However, the parameter passed to this function is the TokenManager address, when the expected parameter is the token address, resulting in TokenManager::_transfer being reverted.
Impact
Unable to transfer tokens from CapitalPool to msg.sender because TokenManager is not approved by CapitalPool. To approve TokenManager for token transfers from CapitalPool, the user must directly call the CapitalPool::approve function from the CapitalPool contract.
Recommendations
Update the code to call the CapitalPool::approve function within the TokenManager::_transfer function:
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.