SafeERC20: Does safeIncreaseAllowance have same security issue as approve?

My contract wants to increase a user’s allowance by +5. I simply call safeIncreaseAllowance(user, 5) as recommended by the SafeERC20 contract.

An auditor suggested the following:
safeIncreaseAllowance(user, 0); safeIncreaseAllowance(user, 5); This does not make sense to me, as the first call does nothing. I think he’s copying the pattern for approve.

Question: Is my call above safe with respect to the known vulnerability to approve?

Discussion:

I read about the vulnerability in approve. It looks to me like most people are still doing this wrong. AFAIK, the “vulnerability” is only when you send 2 separate approve transactions and the attacker front runs the calls to grab more tokens than intended.

For example, yearn’s contracts do the following:

IERC20(want).safeApprove(ypool, 0);
IERC20(want).safeApprove(ypool, _want);

How is this safer? The pair of calls is equivalent to calling approve(ypool, _want). But the pair of calls is used to undermine the safety checks in safeApprove.

1 Like

Exactly!
Setting approve twice is not necessary if you know for sure they will be part of the same tx. This is why you don't need to set approval to 0 before doing an "increaseAllowance".

1 Like

Thanks. The auditor explained why, and now it makes sense. Some ERC20 contracts (e.g. Tether) changed the behavior of approve to look like safeApprove. That means you must call approve(me, 0) and approve(me, value). If you call safeIncreaseAllowance it will fail because approves directly to a new value without going to 0 first.

In fact, this means you can’t use SafeERC20 in a general way because some tokens are broken. We have to modify SafeERC20 to call approve twice. That’s too bad.

1 Like