Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

A wrong address is used during the approval for the Capital Pool

gkrastenov
## Summary
A wrong address is used during the approval for the Capital Pool.
​
## Vulnerability Details
​
When _`from == _capitalPoolAddr` and `IERC20(_token).allowance(_from, address(this)) == 0x0` are true, the `approve` function in the Capital Pool is called, which then calls the `approve` function for the token. The problem arises because the wrong address is used for `tokenAddr`. In this case, `tokenAddr` is incorrectly set to the `TokenManager` contract.
​
```solidity
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
//@audit-issue
ICapitalPool(_capitalPoolAddr).approve(address(this));
}
```
​
Also, the `approve` function of `CapitalPool` contract should only be called by the `TokenManager`. Currently, the function can be called by anyone.
​
## Impact
Blocking the transfer functionality.
​
## Tools Used
Manual Review
​
## Recommendations
Use the token address as the input instead of `address(this)`.
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.