search

code-423n4 / 2022-11-looksrare-findings

0 stars 0 forks source link

Approval mechanism won’t work correctly with USDT and other tokens that have approval race protection #205

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-looksrare/blob/e42ac05b3b740292422a725e9b57687e62d32c67/contracts/lowLevelCallers/LowLevelERC20Approve.sol#L25

Vulnerability details

Proof of Concept

Some tokens (e.g. USDT, KNC) do not allow approving an amount M > 0 when an existing amount N > 0 is already approved. Since the code in LowLevelERC20Approve::_executeERC20Approve does not first approve zero allowance, this functionality will revert with the above mentioned tokens.

Impact

The impact is that calls to _executeERC20Approve will revert when the allowance is non-zero and the token has such a protection mechanism, hence Medium severity

Recommendation

When using ERC20::approve always approve 0 allowance first so the protocol is compatible with USDT and other such tokens

Picodes commented 1 year ago

Finding is valid but in the codebase _executeERC20Approve is only used in a onlyOwner function here so there is no break of functionality: the owner can just approve 0 and reapprove.

c4-sponsor commented 1 year ago

0xhiroshi marked the issue as sponsor disputed

0xhiroshi commented 1 year ago

Invalid: we will just do 2 approvals if needed.

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-11-looksrare-findings/issues/203