sherlock-audit / 2024-11-telcoin-judging

0 stars 0 forks source link

Igdbase_ - H-01 Approval Logic Incompatibility with Tokens that Revert on Zero-Value Approvals #183

Open sherlock-admin2 opened 2 weeks ago

sherlock-admin2 commented 2 weeks ago

Igdbase_

High

H-01 Approval Logic Incompatibility with Tokens that Revert on Zero-Value Approvals

Summary

The protocol’s _swap function currently attempts to reset token allowances to zero before re-approving a new amount. However, certain tokens, such as BNB, revert when a zero-value approval is attempted. This behavior causes a complete failure of any transaction involving these tokens within the protocol, as the _swap function fails when it tries to reset the approval. This issue affects swaps that involve these tokens, breaking protocol functionality for any token that reverts on zero approvals.

Root Cause

Proof of Concept

The vulnerability lies in the _swap function, where the protocol tries to reset approvals for tokens by setting allowances to zero before re-approving. This approach fails for tokens like BNB, which revert on zero-value approvals. As a result, any swap transaction involving such tokens will fail when _swap attempts to reset the approval.

Affected Code:

function _swap(IERC20 tokenIn, IERC20 tokenOut, uint256 amountIn, uint256 amountOutMin, bytes memory swapData) internal returns (uint256 amountInDelta, uint256 amountOutDelta) {
    if (amountIn != 0 && swapData.length != 0 && address(tokenOut) != address(0)) {
        // approve needed amount
        _safeApprove(tokenIn, swapRouter, amountIn);

        // execute swap
        (bool success,) = swapRouter.call(swapData);
        if (!success) {
            revert ("swap failed!");
        }

        // reset approval
        //@audit resetting the approval would never work for these tokens
        _safeApprove(tokenIn, swapRouter, 0);

        // additional processing...
    }
}

Relevant Protocol Documentation:
The protocol explicitly acknowledges that it intends to support tokens expected to behave as standard ERC-20 tokens. However, the use of _safeApprove(tokenIn, swapRouter, 0); in _swap breaks the functionality for any token that reverts on zero-value approvals. This design causes failures specifically for tokens that disallow setting approvals to zero before re-approving.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

The protocol’s _swap function currently attempts to reset token allowances to zero before re-approving a new amount. However, certain tokens, such as BNB, revert when a zero-value approval is attempted. This behavior causes a complete failure of any transaction involving these tokens within the protocol, as the _swap function fails when it tries to reset the approval. This issue affects swaps that involve these tokens, breaking protocol functionality for any token that reverts on zero approvals.

PoC

No response

Mitigation

Recommended Mitigation Steps

To ensure compatibility with tokens that revert on zero-value approvals, consider using a conditional mechanism for resetting allowances. If the zero-value approval fails, use an alternate reset method. Here’s an updated _safeResetAndApprove function that could help in mitigating this issue:

function _safeResetAndApprove(IERC20 token, address _spender, uint256 _value) internal {
    // Attempt to reset allowance to zero; catch any errors
    try token.approve(_spender, 0) {} catch {
        // Handle tokens that revert on zero-approval
    }
    // Set the desired allowance value (non-zero)
    require(_value > 0, "Approval value must be greater than zero");
    _safeApprove(token, _spender, _value);
}

By incorporating _safeResetAndApprove in _swap, the protocol can handle tokens that do not allow zero-value approvals, while ensuring the required allowance for future transfers is set. Alternatively, the protocol could limit support to tokens that do not revert on zero-value approvals.