search

code-423n4 / 2021-09-sushimiso-findings

0 stars 0 forks source link

`_safeApprove()` is Not Used Instead of `approve()` #33

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

Some tokens require resetting approval to 0 before another value in order to prevent frontrunning of the approve() function. Openzeppelin's safeApprove implementation ensures two approve() calls are made, enabling all tokens to be used in MISOMarket.sol and MISOLauncher.

Proof of Concept

https://github.com/sushiswap/miso/blob/master/contracts/MISOMarket.sol#L285 https://github.com/sushiswap/miso/blob/master/contracts/MISOLauncher.sol#L282

Tools Used

Manual code review

Recommended Mitigation Steps

Consider using Openzeppelin's implementation of safeApprove for all approve() calls.

Clearwood commented 3 years ago

Do you have a list of tokens with this problem? In normal cases this is problematic as it leads to increased costs

ghoul-sol commented 3 years ago

making this a non-critical as it is best practices recommendation