`TokenManager::_transfer` function passes token manager address to `CapitalPool::approve` function instead of the token address, allowing transfers to revert
Summary
The CapitalPool::approve stores all the tokens, and is expected to approve the token manager, to allow it to send the tokens to users.
The CapitalPool::approve function expects the token address for which approval is to be done, but TokenManager::_transfer function passes its own address instead of token address making the calls to always revert, and no funds can be transferred from Capital Pool.
Vulnerability Details
The vulnerability is present in the
TokenManager::_transferfunction where it passes incorrect parameter toCapitalPool::approvefunction making the calls to always revert.The CapitalPool::approve expects a token address to allow approvals to the TokenManager, but instead of the token address,
TokenManager::_transferpasses its own contract address, making the call to always revert, as there is no approve function on theTokenManagercontract and this action is totally irrelevant.Thus it makes the functions to revert which depends on
_transfersuch as withdraw.
Note: Even though it calls it when the approval is 0 in the check, and anyone can call the approve function before doing that operation in Capital Pool as it has no access control. But due to the vulnerability where the anyone can frontrun anyone's txn to call the approve function and set the amount to any arbitrary value less than the transfer amount will make the user's withdrawal to revert, therefore an access control of token manager was needed on approve function. So, only token manager can call it.
Impact
_transferfunction will always revert and make all dependent functions to revert, for the case when_fromparameter is capital pool contract.Thus, it makes the
withdrawfunction to revert and user will not be able to withdraw their tokens.
Tools Used
Manual Review
Recommendations
Pass the token address to approve function instead of the token manager address.
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.