Missing validation in Handle name could cause Path traversal attacks or DOS

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/PublishingLogic.sol#L398-L410

Vulnerability details

1. User A creates a new Profile which calls createProfile function on PublishingLogic.sol#L39
2. Assuming that user has provided vars.handle as ..
3. _validateHandle gets called which checks if .. is valid handle

function _validateHandle(string calldata handle) private pure {
        bytes memory byteHandle = bytes(handle);
        if (byteHandle.length == 0 || byteHandle.length > Constants.MAX_HANDLE_LENGTH)
            revert Errors.HandleLengthInvalid();

        for (uint256 i = 0; i < byteHandle.length; ++i) {
            if (
                (byteHandle[i] < '0' ||
                    byteHandle[i] > 'z' ||
                    (byteHandle[i] > '9' && byteHandle[i] < 'a')) && byteHandle[i] != '.'
            ) revert Errors.HandleContainsInvalidCharacters();
        }
    }

4. As we can see in _validateHandle, . is acceptable character. So .. is accepted and _validateHandle returns success

5. A new profile for User A gets created with handle ..

6. Problem occurs in "UI/any app ui using aave lens" since .. is used for parent directory.

7. So basically when user will try accessing his profile .. by ui then it would be like "https://www.something.com/profile/..".

8. This url will simply redirect to https://www.something.com/ and this user will never be able to access his profile on UI

Remediation: Enforce _validateHandle to see that handle contains atleast one letter or number

function _validateHandle(string calldata handle) private pure {
        bytes memory byteHandle = bytes(handle);
        bool letterOrNumFound=false;

        if (byteHandle.length == 0 || byteHandle.length > Constants.MAX_HANDLE_LENGTH)
            revert Errors.HandleLengthInvalid();

        for (uint256 i = 0; i < byteHandle.length; ++i) {
            if (
                (byteHandle[i] < '0' ||
                    byteHandle[i] > 'z' ||
                    (byteHandle[i] > '9' && byteHandle[i] < 'a')) && byteHandle[i] != '.'
            ) revert Errors.HandleContainsInvalidCharacters();

            if(byteHandle[i] != '.'){letterOrNumFound=true;}

        }

        if(!letterOrNumFound){
        revert Errors.HandleContainsInvalidCharacters();
        }

    }

[oneski]

see comment on #65

This is an issue on the FE implementation side, not in the contracts. Contracts function as designed "." and ".." are both valid it is up to governance to not add contracts to the allowlist that will let these profiles be minted.

[0xleastwood]

I agree, this should be handled by the front-end and not on a smart contract level. It isn't ideal to handle input sanitisation on a smart contract level where computation is expensive.