search

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

0 stars 0 forks source link

Inconsistent usage of `safeApprove` #141

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x0x0x

Vulnerability details

In factory safeApprove is used directly without approving 0 first, as seen in:

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

In basket safeApprove is used by first approving 0, then the required amount as seen in:

https://github.com/code-423n4/2021-12-defiprotocol/blob/205d3766044171e325df6a8bf2e79b37856eece1/contracts/contracts/Basket.sol#L276-L277

So factory doesn't support tokens such as USDT, but basket does. It is better to have a general approach to apply safeApprove to make different contracts support same tokens.

frank-beard commented 2 years ago

would consider this a low risk issue

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.