search

code-423n4 / 2021-12-defiprotocol-findings

0 stars 0 forks source link

Use approve(0) first #116

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Jujic

Vulnerability details

Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

Impact

When using one of these unsupported tokens, all transactions revert and the protocol cannot be used.

Proof of Concept

https://github.com/code-423n4/2021-12-defiprotocol/blob/205d3766044171e325df6a8bf2e79b37856eece1/contracts/contracts/Factory.sol#L112

Tools Used

VSC

Recommended Mitigation Steps

Add SafeApprove with a zero amount first before setting the actual amount.

0xleastwood commented 2 years ago

This is not valid. Tokens are being approved on a newly cloned contract called newBasket. The initial approval for this contract would be zero, however, users could potentially send small amounts of USDT to the deterministically created contracts, effectively DoS'ing them. This would only causes issues with baskets which were known to utilise non-standard tokens such as USDT. Although, this was not outlined by the warden.