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

External approve() call will fail due to passing in wrong address

turvec

Summary

External approve call to CapitalPool will fail due to passing in the wrong address causing multiple executions to revert

Context:
TokenManager.sol#L247

Vulnerability Details

The internal _transfer() function in the TokenManager.sol which many functions operations will call with different user tokens, will require getting CapitalPool allowance for transfers from CapitalPool that are not yet approved for that token, it's able to do this by calling the approve function in the CapitalPool contract which should be called by the tokenManager only:

* @notice only can be called by token manager
* @param tokenAddr address of token <@
*/
function approve(address tokenAddr) external {
address tokenManager = tadleFactory.relatedContracts(
RelatedContractLibraries.TOKEN_MANAGER
);
(bool success, ) = tokenAddr.call( <@
abi.encodeWithSelector(
APPROVE_SELECTOR,
tokenManager,
...
}

Notice the function param takes in the token address to make the low level approve call to. However, in the TokenManager.sol::_transfer() function it calls the CapitalPool approve function with it's contract address which is not an erc20 token or implements the token approve function, instead of passing in the token address:

function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
...
​
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
ICapitalPool(_capitalPoolAddr).approve(address(this)); <@
}
...

This will cause the external approve call to fail causing multiple executions to revert

Impact

External approve call will fail causing multiple executions to revert

Tools Used

Manual Review

Recommendations

Fix:

function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
...
​
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
- ICapitalPool(_capitalPoolAddr).approve(address(this));
+ ICapitalPool(_capitalPoolAddr).approve(_token);
}
...
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.