code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/namespaces/LensHandles.sol#L250

Vulnerability details

Impact

Severity: Medium Likelihood: High

Description

The _isAlphaNumeric function in the LensHandles.sol contract does not correctly handle uppercase letters. This function is intended to check if a given string is alphanumeric, but due to the current implementation, it fails to recognize uppercase letters as valid alphanumeric characters. This could lead to unexpected behavior if the function is used to validate user inputs or other strings that may contain uppercase letters.

Tools Used

Manual Code Review

Recommendation

It is recommended to adjust the _isAlphaNumeric function to correctly handle uppercase letters. This could be achieved by converting the input string to lowercase before performing the alphanumeric check, or by adjusting the check itself to include the range of ASCII values for uppercase letters.

Recommended Mitigation Steps

Modify the _isAlphaNumeric function to include uppercase letters in its validation. This could be done by adding an additional condition to check for the ASCII values of uppercase letters. Specifically, the function could be updated as follows:
```
return (char >= '0' && char <= '9') || (char >= 'a' && char <= 'z') ||  (char >= 'A' && char <= 'Z');
```


- After making the necessary changes, thoroughly test the function with a variety of inputs to ensure it behaves as expected. This should include strings with uppercase letters, lowercase letters, numbers, and non-alphanumeric characters.
- Consider adding comments to the function to clearly document its intended behavior and any assumptions it makes about its inputs. This can help prevent similar issues in the future.

## Assessed type

Invalid Validation

vicnaum commented 1 year ago

This behaviour is intended (accepting lowercase only), but we should leave a comment to document this. Issue should be QA.

c4-sponsor commented 1 year ago

vicnaum marked the issue as disagree with severity

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)