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

Some ERC20 Tokens donot support type(uint256).max approval

jennifersun

Summary

In CaptitalPool, some ERC20 tokens do not support type(uint256).max approval. This will cause tokenManager has no allowance to transfer tokens in CaptitalPool and anyone cannot withdraw tokens.

Vulnerability Details

In readme, we notice that the tadle protocol will be compatible with any EVM, any ERC20 Tokens that follow ERC20 standard.
In EIP20 standard(https://github.com/ethereum/ercs/blob/master/ERCS/erc-20.md), we define the approve function :
function approve(address _spender, uint256 _value) public returns (bool success). However, the standard does not mention that the ERC20 token must support type(uint256).max approval.
Actually, some ERC20 tokens don't support type(uint256).max approval, such as UNI, COMP, the approve() will be reverted if the approved value is larger than uint96.
When users want to withdraw funds via TokenManager, TokenManager will transfer funds from CapitalPool to users. This operation can be done if we can call CapitalPool::approve() correctly. However, just like what I mention above, if the token is UNI or COMP, the approve() will be reverted, and users cannot withdraw funds via TokenManager.

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
...
if (_tokenAddress == wrappedNativeToken) {
...
} else {
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
...
}
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();
}
}

Impact

If the token is UNI or COMP, users cannot withdraw funds via TokenManager.

Tools Used

Manual

Recommendations

One easy way is not including UNI or COMP into the whitelist. If we want to be more flexible, we can add another onlyOwner approve(token_address, amount) version. Owners can configure correctly according to different ERC20 Token's characteristic.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

[invalid] finding-CapitalPool-approve-uint256-max

Thanks for flagging, indeed since uint(-1) is representative of max uint256 value, when entering the `if` statement, it will be converted to uint96 max amout, so it will not revert as described. In issue #361, the mockToken utilized does not correctly reflect the below approval behavior. ```Solidity function approve(address spender, uint rawAmount) external returns (bool) { uint96 amount; if (rawAmount == uint(-1)) { amount = uint96(-1); } else { amount = safe96(rawAmount, "Comp::approve: amount exceeds 96 bits"); } ```

Appeal created

kiteweb3 Judge
about 1 year ago
0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-CapitalPool-approve-uint256-max

Thanks for flagging, indeed since uint(-1) is representative of max uint256 value, when entering the `if` statement, it will be converted to uint96 max amout, so it will not revert as described. In issue #361, the mockToken utilized does not correctly reflect the below approval behavior. ```Solidity function approve(address spender, uint rawAmount) external returns (bool) { uint96 amount; if (rawAmount == uint(-1)) { amount = uint96(-1); } else { amount = safe96(rawAmount, "Comp::approve: amount exceeds 96 bits"); } ```

Support

FAQs

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