[WP-N0] Race condition on ERC20 approval

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-insure/blob/19d1a7819fe7ce795e6d4814e7ddf8b8e1323df3/contracts/InsureDAOERC20.sol#L325-L335

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.

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).

[0xHaku]

@oishun1112 what error message do we set at require(allowed[msg.sender][_spender] == 0).?

[oishun1112]

we don't implement this to avoid incompatibility with other project. we implement and prefer to use increase/decreaseAllowance() usually