Missing slippage protection in functions buyCurvesToken() and sellCurvesToken()

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L211

Vulnerability details

Impact

Functions buyCurvesToken() and sellCurvesToken() are used to buy and sell curves token shares of a specific tokenSubject in exchange for ETH. The issue with the functions is that they're missing slippage parameters. Due to this, the functions are prone to MEV attacks such as sandwiching and unintentional frontrunning by other users, which causes them to receive a worse execution price.

Additionally due to the slippage, there can be unintentional extra ETH sent by the user (if someone sells right before user), which remains permanently locked in the Curves contract. This occurs because this check in _buyCurvesToken() is not a strict "==", which allows acceptance of more ETH than intended.

Proof of Concept

Here is the whole process:

1. Charlie observes the price for token subject alice using getBuyPriceAfterFee
2. Charlie decides to buy shares of alice but bob frontruns charlie's transaction by calling buyCurvesToken() first and paying more gas. (Note: Bob can also just be a regular user whose tx gets executed first)
3. Charlie's transaction goes through but with a worse execution price.

Coded POC

Here is how to run this POC:

• Integrate foundry into the project through this
• Add the POC in the test folder.
• Run forge test --match-test testSlippageIssue -vvv

File: testCurvesSlippage.t.sol
01: // SPDX-License-Identifier: MIT
02: pragma solidity ^0.8.7;
03: 
04: import {Curves} from "../contracts/Curves.sol";
05: import {CurvesERC20Factory} from "../contracts/CurvesERC20Factory.sol";
06: import {FeeSplitter} from "../contracts/FeeSplitter.sol";
07: import {Test} from "forge-std/Test.sol";
08: import {console} from "forge-std/console.sol";
09: 
10: contract testCurves is Test {
11: 
12:     // Contract instances
13:     Curves curves;
14:     CurvesERC20Factory factory;
15:     FeeSplitter fs;
16: 
17:     // Fee destinations
18:     address public protocolFeeDestination = makeAddr("protocolFeeDestination");
19:     address public referralFeeDestination = makeAddr("referralFeeDestination");
20:  
21:     // Token subject
22:     address public alice = makeAddr("alice");
23: 
24:     // Sample user
25:     address public bob = makeAddr("bob");
26:     address public charlie = makeAddr("charlie");
27: 
28:     function setUp() public {
29:         factory = new CurvesERC20Factory();
30:         fs = new FeeSplitter();
31:         curves = new Curves(address(factory), address(fs));
32:     }
33: 
34:     function testSetupFees() public {
35:         // 20% - Protocol fees
36:         // 20% - Subject fees
37:         // 10% - Referral fees
38:         // 50% - Holder fees
39:         curves.setMaxFeePercent(1e18);
40:         curves.setProtocolFeePercent(2e17, protocolFeeDestination); 
41:         curves.setExternalFeePercent(2e17, 1e17, 5e17);
42:         vm.prank(alice);
43:         curves.setReferralFeeDestination(alice, referralFeeDestination);
44:     }
45: 
46:     function testBuyFirstCurveToken() public {
47:         vm.prank(alice);
48:         curves.buyCurvesToken(alice, 1);
49:     }
50: 
51:     function testSlippageIssue() public {
52:         testSetupFees();
53: 
54:         fs.setCurves(Curves(address(curves)));
55:         testBuyFirstCurveToken();
56: 
57:         // Charlie observes price for token subject alice and decides to buy shares
58:         vm.prank(charlie);
59:         uint256 buyPriceAfterFeeForCharlie = curves.getBuyPriceAfterFee(alice, 99);
60:         console.log("Charlie's expected price:", buyPriceAfterFeeForCharlie);
61: 
62:         // Bob's transaction frontruns Charlie's transaction
63:         uint256 buyPriceAfterFeeForBob = curves.getBuyPriceAfterFee(alice, 99);
64:         vm.deal(bob, buyPriceAfterFeeForBob);
65:         vm.prank(bob);
66:         curves.buyCurvesToken{value: buyPriceAfterFeeForBob}(alice, 99);
67: 
68:         // Charlie's transaction goes through after Bob's
69:         buyPriceAfterFeeForCharlie = curves.getBuyPriceAfterFee(alice, 99);
70:         console.log("Charlie's execution price:", buyPriceAfterFeeForCharlie);
71:         vm.deal(charlie, buyPriceAfterFeeForCharlie);
72:         vm.prank(charlie);
73:         curves.buyCurvesToken{value: buyPriceAfterFeeForCharlie}(alice, 99);
74:     }
75: }

Slippage issue confirmation

Tools Used

Manual Review

Recommended Mitigation Steps

Add a slippage parameter in both functions to minimize this issue.

Assessed type

MEV

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #13

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid