DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

The safeApprove in ParaSwapUtils may revert

Summary

The safeApprove function in the OpenZeppelin SafeERC20 library is designed to safely approve a spender to transfer tokens on behalf of the owner. However, improper usage of safeApprove can lead to vulnerabilities, particularly when dealing with ERC20 tokens that do not reset the allowance to zero before setting a new value. This can result in race conditions, front-running attacks, or unexpected behavior if the allowance is not managed correctly.

Vulnerability Details

The swap function in ParaSwapUtils uses ERC20 safeApprove() from OpenZeppelin's SafeERC20 library to give allowance to the paraswap:

function swap(address to, bytes memory callData) external {
_validateCallData(to, callData);
address approvalAddress = IAugustusSwapper(to).getTokenTransferProxy();
address fromToken;
uint256 fromAmount;
assembly {
fromToken := mload(add(callData, 68))
fromAmount := mload(add(callData, 100))
}
IERC20(fromToken).safeApprove(approvalAddress, fromAmount);
(bool success, ) = to.call(callData);
require(success, "paraswap call reverted");
}

However, the safeApprove function prevents changing an allowance between non-zero values to mitigate a possible front-running attack. It reverts if that is the case. Comment from the OZ library for this function():

/**
* @dev Deprecated. This function has issues similar to the ones found in
* {IERC20-approve}, and its usage is discouraged.
*
* Whenever possible, use {safeIncreaseAllowance} and
* {safeDecreaseAllowance} instead.
*/
function safeApprove(IERC20 token, address spender, uint256 value) internal {
// safeApprove should only be called when setting an initial allowance,
// or when resetting it to zero. To increase and decrease it, use
// 'safeIncreaseAllowance' and 'safeDecreaseAllowance'
require(
(value == 0) || (token.allowance(address(this), spender) == 0),
"SafeERC20: approve from non-zero to non-zero allowance"
);
_callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
}

If the existing allowance is non-zero, then safeApprove() will revert causing swap to fail and leading to denial-of-service.

Impact

The safeApprove() may revert causing swap to fail and leading to denial-of-service. The impact is Medium, the likelihood is Low, so the severity is Low.

Tools Used

Manual Review

Recommendations

Consider using forceApprove instead().

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_safeApprove_no_reset

USDT or other unusual ERC20 tokens: out of scope. For the other reports: No proof that the allowance won't be consumed by the receiver.

Support

FAQs

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