Incorrect check in `isValidName` function

[hats-bug-reporter[bot]]

Github username: @Jelev123 Twitter username: zhulien_zhelev Submission hash (on-chain): 0x15612199e179e17a371d8bc666539cfdf9f8a9b5b891b0972a4e441e7d09ac82 Severity: low

Description: Description\ In isValidName function in loop has a check for allowed characters. The check is incorrect because if one if the conditions is false it will return true which will be invalid in this case.

`The allowed characters: 0-9, A-Z, a-z, space,

• hyphen, underscore, period, parentheses, apostrophe,
• ampersand, plus and hash.` If some of the characters is valid the function will return true, thats mean the name will have just one valid character.

1. Proof of Concept (PoC) File

   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;
   }

Recommendation

Use | | operator instead && like it done in isValidSymbol function

[0xmahdirostami]

It's correct, you could test it.

it is checking that if a char will not any of those, then return false.

    function testisvalid() public {
        string memory notValid = "??Mahdi";
        mockNameRegistry.isValidName(notValid);
    }

[PASS] testisvalid() (gas: 7288)
Traces:
  [7288] NamesTest::testisvalid()
    ├─ [1768] MockNameRegistry::isValidName("??Mahdi") [staticcall]
    │   └─ ← [Return] false
    └─ ← [Stop] 

[Jelev123]

@0xmahdirostami You're right, I was mistaken

[benjaminbollen]

Thank you for your report on the isValidName function. After review, we've determined this is not an issue.

The isValidName and isValidSymbol functions are logically equivalent, despite their different implementations. This equivalence is based on De Morgan's laws in Boolean algebra.

We appreciate your thorough examination of our code and attention to logical operations. Your diligence contributes to the quality of our codebase. Thank you for your valuable input.