Inconsistency in Custom Name and Symbol Registration Functions Leads to Unexpected Reverts

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xfbb00fba43c02e52b4e45bfd463e4bbcb2aeb9e5e90c5884d320613ea3903ab3
Severity: medium

Description:
Description
In both the registerCustomName and registerCustomSymbol functions of the NameRegistry contract, there are critical inconsistencies between the comments and the implementations.

The comments suggest that when empty strings are provided, they should default to specific values.

However, the functions attempt to validate these empty strings, which fail the isValidName and isValidSymbol checks respectively, causing the functions to revert.

For registerCustomName, the comment states:

// if name is left empty, it will default to default name "Circles-<base58(short)Name>"

For registerCustomSymbol, the comment states:

// if symbol is left empty, it will default to default symbol "CRC"

However, in both cases, empty strings lead to function reverts rather than setting default values.

[benjaminbollen]

Not even false. If empty, returns; if not empty but invalid, reverts.

[benjaminbollen]

but a wrong line comment wouldnt even qualify for a low issue, let alone medium

[0xvivekd]

    function registerCustomName(address _avatar, string calldata _name) external onlyHub(1) {
        if (bytes(_name).length == 0) {
            // if name is left empty, it will default to default name "Circles-<base58(short)Name>"
            return;
        }
        if (!isValidName(_name)) {
            revert CirclesNamesInvalidName(_avatar, _name, 0);
        }
        customNames[_avatar] = _name;
    }

```javascript
function isValidName(string calldata _name) public pure returns (bool) {
    bytes memory nameBytes = bytes(_name);
    if (nameBytes.length > 32 || nameBytes.length == 0) return false; // Check length

    for (uint256 i = 0; i < nameBytes.length; i++) {
        bytes1 char = nameBytes[i];
        if (
            !(char >= 0x30 && char <= 0x39) // 0-9
                && !(char >= 0x41 && char <= 0x5A) // A-Z
                && !(char >= 0x61 && char <= 0x7A) // a-z
                && !(char == 0x20) // Space
                && !(char == 0x2D || char == 0x5F) // Hyphen (-), Underscore (_)
                && !(char == 0x2E) // Period (.)
                && !(char == 0x28 || char == 0x29) // Parentheses ( () )
                && !(char == 0x27) // Apostrophe (')
                && !(char == 0x26) // Ampersand (&)
                && !(char == 0x2B || char == 0x23) // Plus (+), Hash (#)
        ) {
            return false;
        }
    }
    return true;
}

   
    
    The isVaildName reverts if length is 0 due to the below line:
    
    ```javascript
    if (nameBytes.length > 32 || nameBytes.length == 0) return false; // Check length

[benjaminbollen]

right, you confirm my point. Thanks for your contribution

[0xvivekd]

Could you explain what is the intention?

Empty strings in name and symbol should be allowed or not?

[0xvivekd]

It returns false and reverts with CirclesInvalidString

Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf/src/hub/Hub.sol

Lines 1017 to 1026 in 507e185

	if (!nameRegistry.isValidName(_name)) {
	// Invalid group name.
	// name must be ASCII alphanumeric and some special characters
	revert CirclesInvalidString(_name, 0);
	}
	if (!nameRegistry.isValidSymbol(_symbol)) {
	// Invalid group symbol.
	// symbol must be ASCII alphanumeric and some special characters
	revert CirclesInvalidString(_symbol, 1);
	}

[0xvivekd]

I have resubmitted the issue in Issue # 110

Kindly ignore this issue

[benjaminbollen]

this is different from #110. Here it seems you confuse the behaviour of isValidName with registerCustomName which are two different functions.

In #110, you say a group must be allowed to have an empty name, which they they are not allowed