code-423n4 / 2022-02-aave-lens-findings

0 stars 0 forks source link

Profile can be created with a handle string "." #65

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

The current implementaion of the validateHandle is very basic, and allows for handles like a single "." or series of "...." as valid values. There should be more checks added like, the handle should not start with '.' or end with '.' or only have '.' in the handle.

The bigger issue is that there is no standard being defined/followed for the handle. It a very basic check of length, smallcaps, etc.,

Impact

The handle parameter is an important entity similar to primary key in a database. There is no standard being followed for this parameter, and may need to be redefined in future, which may pose challanges for backward compatibility.

Filing this issue as Medium because the current convention used is very basic and not future proof.

Proof of Concept

Contract : PublishingLogic.sol Function : function _validateHandle(string calldata handle)

Recommended Mitigation Steps

1) Follow some standard like the ENS, which stores the hash of the domain (similar to handle) on the chain. OR, 2) Follow some standards which gives an identity, similar to email addresses. OR, 3) Another suggestion is to include @ and _ as valid special characters, and have more checks like should not have more than one @ in the handle, etc.,

oneski commented 2 years ago

declined, this is as designed. Profile allowlist allows governance to only allow contracts that will mint within certain specs. Protocol itself doesn't have an opinion on valid or invalid names.

0xleastwood commented 2 years ago

As per the sponsor's comment, making this as invalid.