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

Incompatibility with Some ERC20 Standard Tokens that Revert on Large Approvals

aresaudits

Summary

Vulnerability Details

/**
* @dev Approve token for token manager
* @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,
type(uint256).max
)
);
​
if (!success) {
revert ApproveFailed();
}
}
​

The approve function in CapitalPool.sol attempts to approve the maximum possible uint256 value (type(uint256).max) for the token manager. While this approach is generally compatible with standard ERC-20 tokens, it can cause issues with certain erc20 Standard tokens like UNI and COMP, which have special logic to handle large approval values.

Special Case Logic in UNI and COMP:

  • Both UNI and COMP tokens revert if the value passed to approve is larger than uint96.

  • These tokens have special case logic in their approve function that sets the allowance to type(uint96).max if the approval amount is uint256(-1) (equivalent to type(uint256).max).

- submitting this issue as medium because this function is called in multiple critical functions like withdraw etc

Impact

If the approve function in CapitalPool.sol is called with UNI or COMP tokens, the transaction will revert due to the large approval value.

Tools Used

Manual Review

Recommendations

To address this issue, the approve function should be modified to handle tokens that revert on large approvals. One approach is to check the token type and adjust the approval value accordingly

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 over 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!