Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

tokenManager::withdraw() will revert if withdraw asset's approve method doesn't return a bool

0xaraj

Summary

tokenManager::withdraw() will revert if withdraw asset's approve method doesn't return a bool

Vulnerability Details

A user can withdraw his tokens from capitalPool using tokenManager:withdraw(). If _from = capitalPool & allowance is 0 then it calls the ERC20::approve method, which is expected to return a bool value

if (_from == _capitalPoolAddr && IERC20(_token).allowance(_from, address(this)) == 0x0) {
@> ICapitalPool(_capitalPoolAddr).approve(_token);
}
function approve(address tokenAddr) external {
address tokenManager = tadleFactory.relatedContracts(RelatedContractLibraries.TOKEN_MANAGER);
@> (bool success,) = tokenAddr.call(abi.encodeWithSelector(APPROVE_SELECTOR, tokenManager, type(uint256).max));
​
if (!success) {
revert ApproveFailed();
}
}

Using ERC20::approve will not work with ERC20 tokens that do not return a bool (e.g., USDT).
Solidity has return data length checks, and if the token implementation does not return a bool value, the transaction will revert.

//Here is PoC
Run test with forge test --mc POC --rpc-url=<mainnet-rpc-url> -vv

// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;
​
import {Test, console2} from 'forge-std/Test.sol';
import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
​
contract POC is Test {
address constant USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7;
address immutable owner = makeAddr("owner");
address immutable spender = makeAddr("spender");
​
function setUp() external {
deal(USDT, owner, 1e6);
}
​
function testApproveRevert() external {
vm.prank(owner);
IERC20(USDT).approve(spender, 1e6);
}
​
function testApproveSuccess() external {
vm.prank(owner);
SafeERC20.forceApprove(IERC20(USDT), spender, 1e6);
​
uint256 allowance = IERC20(USDT).allowance(owner, spender);
assertEq(allowance, 1e6);
}
}

Impact

Token of users will stuck in capitalPool as approve() will revert

Tools Used

Manual Review

Recommendations

Use forceApprove from OpenZeppelin's SafeERC20 library.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic
Assigned finding tags:

[invalid] finding-CapitalPool-approve-return-boolean

Invalid, low level call will always return true as long as the call succeeds without reverting, so this has no impact described, given approvals can only fail when some weird tokens do not allow a uint256.max approval, which is not described in any of the issues below.

Support

FAQs

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

Give us feedback!