Approval Race Condition for USDT Token

Summary

Vulnerability Detail

The current implementation of the approve function in Position.sol does not handle the approval race condition for tokens like USDT, which require resetting the allowance to zero before setting a new value. This can cause transactions to revert when attempting to approve a new allowance for USDT.

  /// @notice Approve an external contract to spend funds from the position
    /// @dev The position manager imposes additional checks that the spender is trusted
    function approve(address token, address spender, uint256 amt) external onlyPositionManager {
        // use forceApprove to handle tokens with non-standard return values
        // and tokens that force setting allowance to zero before modification
        IERC20(token).forceApprove(spender, amt);
    }

    /// @dev Approve a spender to use assets from a position
    function approve(address position, bytes calldata data) internal {
        // data -> abi.encodePacked(address, address, uint256)
        // spender -> [0:20] address to be approved
        // asset -> [20:40] address of token to be approves
        // amt -> [40:72] amount of asset to be approved
        address spender = address(bytes20(data[0:20]));
        address asset = address(bytes20(data[20:40]));
        uint256 amt = uint256(bytes32(data[40:72]));

        if (!isKnownAsset[asset]) revert PositionManager_ApproveUnknownAsset(asset);
        if (!isKnownSpender[spender]) revert PositionManager_UnknownSpender(spender);

        // if the passed amt is type(uint).max assume approval of the entire balance
        if (amt == type(uint256).max) amt = IERC20(asset).balanceOf(position);

        Position(payable(position)).approve(asset, spender, amt);
        emit Approve(position, msg.sender, spender, asset, amt);
    }

Impact

transaction reverts when attempting to approve a new allowance for USDT.

Code Snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/Position.sol#L86C4-L92C6

Tool used

Manual Review

Recommendation

Modify the approve function in Position.sol to reset the allowance to zero before setting the new allowance. This ensures compatibility with tokens like USDT.

Duplicate of #48

sherlock-audit / 2024-08-sentiment-v2-judging

AresAudits - Approval Race Condition for USDT Token #593