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

0 stars 0 forks source link

Frontrunning on the low level mint() call is possible in .finalize() #146

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

Suppose you fix the last problem I submitted, so that only the owner can call finalize(). There is another problem as the transaction can be frontrun by an attacker listening in on the mempool.

A similar setup to the previous submission: Suppose an attacker somehow gets a small amount of the auction token before the token (many projects do small token giveaways to contributors, community members, nft artists, code423n4 auditors, etc.) before the official launch.

The attacker writes a bot to listen to the mempool for incoming transactions that use the finalize() method on your contract. Upon finding the incoming transaction, the attacker can continue as normal, frontrunning then backrunning the transaction in the previously described way:

The attacker creates a massively skewed pool for the token. Since you are using the low level .mint() call in the code, "slippage" on your mint could be very high. The attacker lets you use the .finalize method (giving you close to 0 LP tokens because of the skewed pool) and then .burns() his liquidity taking almost the entire pool on his burn.

Proof of Concept

https://github.com/sushiswap/miso/blob/2cdb1486a55ded55c81898b7be8811cb68cfda9e/contracts/Liquidity/PostAuctionLauncher.sol#L220-L222

Recommended Mitigation Steps

Don't use the low level mint call without adding a minimumOut variable to your function and require that you get at least minimumOut LP tokens.

Clearwood commented 3 years ago

duplicate of #14

ghoul-sol commented 2 years ago

This exploits the same vulnerability as in #145 and for that reason it's a duplicate from the same warden so it's invalid.