Not authorizing the token `name` will cause possible phishing attacks

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L428-L437 https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L439-L454 https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L456-L463 https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L338-L356

Vulnerability details

Impact

Note: Due to that the protocol is a Social App, I have decided to report this issue, You have not to compare it with other cases/protocols.

setNameAndSymbol and _deployERC20 are not checking if the name is previously used by another token or not, this let's attacker to create a token with a duplicate name, where the token-name belongs to a famous person, this opens possible phishing attacks for abusers.

Proof of Concept

Bob decides to join into the protocol, so he calls buyCurvesTokenWithName:

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

        _buyCurvesToken(curvesTokenSubject, amount);
        _mint(curvesTokenSubject, name, symbol);
    }

For the name he enters exactly a token-name which belongs to a famous man, and for symbol he enters a unique symbol (or Bob may enter also the exact symbol with 1 different character which make the symbol to be unique, this helps Bob to implement a more successful attack and seems real).

_buyCurvesToken will give the first 1-ether of Bob's token to himself. then it goes to _mint:

function _mint(
        address curvesTokenSubject,
        string memory name,
        string memory symbol
    ) internal onlyTokenSubject(curvesTokenSubject) {
        if (externalCurvesTokens[curvesTokenSubject].token != address(0)) revert ERC20TokenAlreadyMinted();
        _deployERC20(curvesTokenSubject, name, symbol);
    }

It just checks if the Bob already created any curve or not, because the Bob is a new creator/user so he doesn't have any subjectToken and it goes to _deployERC20:

function _deployERC20(
        address curvesTokenSubject,
        string memory name,
        string memory symbol
    ) internal returns (address) {
        // If the token's symbol is CURVES, append a counter value
        if (keccak256(bytes(symbol)) == keccak256(bytes(DEFAULT_SYMBOL))) {
            _curvesTokenCounter += 1;
            name = string(abi.encodePacked(name, " ", Strings.toString(_curvesTokenCounter)));
            symbol = string(abi.encodePacked(symbol, Strings.toString(_curvesTokenCounter)));
        }

        if (symbolToSubject[symbol] != address(0)) revert InvalidERC20Metadata();

        address tokenContract = CurvesERC20Factory(curvesERC20Factory).deploy(name, symbol, address(this));

        externalCurvesTokens[curvesTokenSubject].token = tokenContract;
        externalCurvesTokens[curvesTokenSubject].name = name;
        externalCurvesTokens[curvesTokenSubject].symbol = symbol;

        externalCurvesToSubject[tokenContract] = curvesTokenSubject;

        symbolToSubject[symbol] = curvesTokenSubject;

        emit TokenDeployed(curvesTokenSubject, tokenContract, name, symbol);
        return address(tokenContract);
    }

We see the function is not authorizing the name is previously used or not, and just authorizes the symbol, so all the checks will be passed and the Bob's token will be created.

Now he implements a phishing attack by advertising to other people, and only because of the similarity in token names they may buy the Bob's token instead of the original one.

Tools Used

Manual Review

Recommended Mitigation Steps

Contract should revert if the name is previously used by another token.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #21

[c4-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

QA low.

[andresaiello]

Intended design but we can consider

[c4-sponsor]

andresaiello (sponsor) acknowledged

[c4-judge]

alcueca marked the issue as duplicate of #21

[c4-judge]

alcueca marked the issue as satisfactory