Lack of slippage protection in Curves contract

[c4-bot-2]

Lines of code

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

Vulnerability details

Due to the _buyCurvesToken and sellCurvesToken functions, when executed, if the price is not equal to the expected price (calculated before sending the transaction), two bad scenarios arise:

• Price decrease before the trader's buy transaction is executed:

  The trader loses the additional value paid, which becomes locked in the contract since the contract does not refund the excess amount.

• Price decrease before the trader's sell transaction is executed:

  The trader sells their tokens at a lower price than expected.

Impact

• Extra paid value remains locked, resulting in financial loss for the trader.
• The trader sells their tokens at an unexpected price

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.7;

import "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {CurvesERC20} from "@contracts/CurvesERC20.sol";
import {CurvesERC20Factory} from "@contracts/CurvesERC20Factory.sol";
import {FeeSplitter} from "@contracts/FeeSplitter.sol";
import {Curves} from "@contracts/Curves.sol";

contract CurvesPOC is Test {
    CurvesERC20 public curvesERC20;
    CurvesERC20Factory public curvesERC20Factory;
    Curves public curves;
    FeeSplitter public feeSplitter;

    // Addresses
    address public owner = makeAddr("OWNER");
    address public protocolFeeDestination = makeAddr("PROTOCOL_FEE_DESTINATION");
    address public attacker = makeAddr("ATTACKER");
    address public curvesSubject = makeAddr("CURVES_SUBJECT");
    address public alice = makeAddr("ALICE");
    address public bob = makeAddr("BOB");

    function setUp() public {
        vm.startPrank(owner);

        // Deploy contracts by owner
        curvesERC20 = new CurvesERC20("Curves", "CURVES", owner);
        curvesERC20Factory = new CurvesERC20Factory();
        feeSplitter = new FeeSplitter();
        curves = new Curves(address(curvesERC20Factory), address(feeSplitter));
        feeSplitter.setManager(address(curves), true);

        // Change curve address
        feeSplitter.setCurves(curves);
        assert(feeSplitter.curves() == curves);

        // Changing fee percent (sample values)
        curves.setMaxFeePercent(1e18);
        curves.setProtocolFeePercent(1e17, protocolFeeDestination);
        curves.setExternalFeePercent(1e17, 1e17, 7e17);

        vm.stopPrank();
    }

    function test_traderCanLossMoneyWithoutSlippageProtection() public {
        // Funding users
        vm.deal(alice, 100 ether);
        vm.deal(bob, 100 ether);

        console.log("Contract balance at start: ", address(feeSplitter).balance);

        // Curve token subject create token
        vm.prank(curvesSubject);
        curves.buyCurvesTokenWithName(curvesSubject, 1, "CURVES_SUBJECT", "CURVES_SUBJECT_SYMBOL");

        // Mempool or sequencer Simulation
        // 1. Bob buy 10 curves token
        // 2. Alice calculate buy 1 curves token cost
        // 3. Bob sell 5 curves token
        // 4. Alice buy 1 curves token by calculated cost

        // Bob buy 1 curves token
        uint256 bob_amount = 10;
        uint256 bob_cost = curves.getBuyPriceAfterFee(curvesSubject, bob_amount);
        vm.prank(bob);
        curves.buyCurvesToken{value: bob_cost}(curvesSubject, bob_amount);

        // Alice calculate buy 1 curves token cost
        uint256 alice_amount = 1;
        uint256 alice_cost = curves.getBuyPriceAfterFee(curvesSubject, alice_amount);
        console.log("Expected buy price: ", alice_cost);

        // Bob sell 5 curves token
        uint256 bob_amount_sell = 5;
        uint256 bob_cost_sell = curves.getSellPriceAfterFee(curvesSubject, bob_amount);
        vm.prank(bob);
        curves.sellCurvesToken(curvesSubject, bob_amount_sell);

        // Alice buy 1 curves token by calculated cost
        uint256 executed_price = curves.getBuyPriceAfterFee(curvesSubject, alice_amount);
        console.log("Executed buy price: ", executed_price);
        vm.prank(alice);
        curves.buyCurvesToken{value: alice_cost}(curvesSubject, alice_amount);

        console.log("Lost: ", alice_cost - executed_price);

        // Alice executed price can just 10% (for example) lower or upper than expected price
        assertApproxEqRel(executed_price, alice_cost, 1e17); // reverted

        // Alice must only pay executed price but contract doesn't refund extra value.
        assertEq(100 ether - executed_price, alice.balance); // reverted
    }
}

POC setup

foundry.toml:

[profile.default]
src = 'contracts'
out = 'out'
libs = ['node_modules', 'lib']
test = 'test/foundry/'
cache_path  = 'cache_forge'
solc_version = "0.8.7"
remappings = [
    "@forge-std/=lib/forge-std/",
    "@openzeppelin/=node_modules/@openzeppelin/",
    "@contracts/=contracts/"
]

forge test --mt test_traderCanLossMoneyWithoutSlippageProtection

Tools Used

Manual Review Foundry

Recommended Mitigation Steps

Implement slippage protection in sellCurvesToken and _buyCurvesToken.

Assessed type

Other

[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