code-423n4 / 2022-01-timeswap-findings

2 stars 0 forks source link

Race condition on ERC20 approval #168

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/base/ERC20.sol#L58-L66

function _approve(
    address owner,
    address spender,
    uint256 amount
) internal {
    allowance[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).

Mathepreneur commented 2 years ago

We choose not to add this fix.