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

CapitalPool's use of type(uint256).max for token approvals may cause issues with ERC20 tokens that revert on large values

krisp

Summary

The CapitalPool contract has a critical issue concerning its handling of ERC20 tokens that revert on large approvals and transfers. Some ERC20 tokens, such as UNI and COMP, revert when the value passed to approve or transfer exceeds uint96. The CapitalPool contract uses type(uint256).max for approvals, which can lead to unexpected behavior with these tokens.

Vulnerability Details

The CapitalPool#approve() function is designed to grant approval to the TokenManager contract for token transfers. It uses type(uint256).max as the approval amount:

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();
}
}

However some ERC20 tokens, such as UNI and COMP, have special logic that handles large approval values. For these tokens, using type(uint256).max could lead to issues.

Impact

The issue causes the CapitalPool contract to potentially malfunction with ERC20 tokens that revert on large approval values. This could lead to failures in granting approvals, causing withdrawal and transfer operations to fail.

Recommendations

Rather than using type(uint256).max, allow specifying a uint96 amount parameter in the approve function. This change ensures compatibility with tokens that have limitations on approval amounts.

function approve(address tokenAddr, uint96 amount) external {
address tokenManager = tadleFactory.relatedContracts(RelatedContractLibraries.TOKEN_MANAGER);
(bool success,) = tokenAddr.call(abi.encodeWithSelector(APPROVE_SELECTOR, tokenManager, amount));
if (!success) {
revert ApproveFailed();
}
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 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
over 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.

Give us feedback!