Race condition on ERC20 approval

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-nftx/blob/194073f750b7e2c9a886ece34b6382b4f1355f36/nftx-protocol-v2/contracts/solidity/token/ERC20Upgradeable.sol#L292-L298

function _approve(address owner, address spender, uint256 amount) internal virtual {
    require(owner != address(0), "ERC20: approve from the zero address");
    require(spender != address(0), "ERC20: approve to the zero address");

    _allowances[owner][spender] = amount;
    emit Approval(owner, spender, amount);
}

Using approve() to manage allowances opens yourself and users of the token up to frontrunning.
Best practice, but doesn't usually matter.

Explanation of this possible attack vector

See also: 0xProject/0x-monorepo#850

A potential fix includes preventing a call to approve if all the previous tokens are not spent through adding a check that the allowed balance is 0:

require(allowed[msg.sender][_spender] == 0).

[dmvt]

duplicate #134