hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

approve() won't work for some collateral tokens #137

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @cpp-phoenix Twitter username: 0xrochimaru Submission hash (on-chain): 0xe886fde4ed2ba90fbb7f19db90e1826e49ddd7c40d7f1edb1a10758fb68f5b8d Severity: medium

Description: Description\ For collateral token like USDT forceApprove() should be used. As USDT require the allowance to be set as 0 before setting it to another value.

Attack Scenario\ The following line of code won't work in case the collateral Token is USDT. forceApprove() should be used to avoid such cases.

collateralToken.approve(address(conditionalTokens), amount);

Implementation of forceApprove() method is SafeERC20.sol contract:

function forceApprove(IERC20 token, address spender, uint256 value) internal {
    bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value));

    if (!_callOptionalReturnBool(token, approvalCall)) {
        _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0)));
        _callOptionalReturn(token, approvalCall);
    }
}
clesaege commented 1 month ago

As per competition rules, are excluded:

cpp-phoenix commented 1 month ago

forceApprove() checks for both cases. It extends the support and provide safeguard for unwanted outcomes. It's similar to using 'safeTransfer()' vs 'transfer()'. In future if the new collateral tokens are supported then this change should be considered too.

clesaege commented 1 month ago

We don't plan on supporting non standard ERC20 tokens.