OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.43k stars 11.68k forks source link

Feature request: utility function to check if a string contains only alphanumeric characters #5087

Open PaulRBerg opened 1 week ago

PaulRBerg commented 1 week ago

🧐 Motivation

Onchain generation of NFT SVGs is on the rise. Many SVGs rely on third-party string data, e.g. ERC-20 symbols.

To sanitize strings and prevent XSS attacks, developers should only allow alphanumeric strings in the token symbol[^1]. This should be enough, since the vast majority of tokens don't contain any special symbols.

It would thus be helpful to have a utility function in OpenZeppelin for checking whether a string contains only alphanumeric characters.

📝 Example Implementation

/// @notice Checks whether the provided string contains only alphanumeric characters and spaces.
/// @dev Note that this returns true for empty strings, but it is not a security concern.
function isAlphanumeric(string memory str) internal pure returns (bool) {
    // Convert the string to bytes to iterate over its characters.
    bytes memory b = bytes(str);

    uint256 length = b.length;
    for (uint256 i = 0; i < length; ++i) {
        bytes1 char = b[i];

        // Check if it's a space or an alphanumeric character.
        bool isSpace = char == 0x20; // space
        bool isDigit = char >= 0x30 && char <= 0x39; // 0-9
        bool isUppercase = char >= 0x41 && char <= 0x5A; // A-Z
        bool isLowercase = char >= 0x61 && char <= 0x7A; // a-z
        if (!(isSpace || isDigit || isUppercase || isLowercase)) {
            return false;
        }
    }
    return true;
}

[^1]: See, for example, finding M-01 in Sablier's recent audit contest on CodeHawks.

PaulRBerg commented 1 week ago

Alternatively, a utility function to check if a single character is alphanumeric would also be helpful:

function isAlphanumericChar(bytes1 char) internal pure returns (bool) {
    bool isSpace = char == SPACE;
    bool isDigit = char >= ZERO && char <= NINE;
    bool isUppercaseLetter = char >= A && char <= Z;
    bool isLowercaseLetter = char >= a && char <= z;
    return isSpace || isDigit || isUppercaseLetter || isLowercaseLetter;
}
Amxx commented 1 week ago

Hello @PaulRBerg Can you give more details as to why this check would be performed onchain, and not offchain by whoever does the call?

PaulRBerg commented 1 week ago

Good point.

NFT UIs should definitely be aware of the possibility of XSS attacks, but I also find it helpful to add an onchain check to minimize the potential harm.