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

0 stars 0 forks source link

User can impersonate other user identity by capturing handle #32

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Duplicate NFT with same name and symbol can be created which could confuse users.

This causes confusion among users who might now follow User B thinking him as User A. If User A was famous, most people will start following User B (who has User A handle) which earns User B money (from follow module)

Proof of Concept

  1. User A with handle abc decides to burn his profile using burn function at LensHub.sol#L493

  2. This calls the _clearHandleHash function

function _clearHandleHash(uint256 profileId) internal {
        bytes32 handleHash = keccak256(bytes(_profileById[profileId].handle));
        _profileIdByHandleHash[handleHash] = 0;
    }
  1. Assume handleHash was X for this User abc, so this sets _profileIdByHandleHash[X] = 0;

  2. User B creates a new profile with vars.handle as abc using createProfile function at LensHub.sol#L142

  3. This calls createProfile at PublishingLogic.sol#L39

  4. Now since _profileIdByHandleHash[handleHash] = 0, so handle abc is accepted and User B now holds handle abc

  5. Now since both NFT symbol and name is generated from handle, User B will generate all NFT with same name and symbol which earlier User A NFT provided

  6. This causes confusion among users who might now follow User B thinking him as User A. If User A was famous, most people will start following User B (who has User A handle) which earns User B money (from follow module)

Recommended Mitigation Steps

Make a global variable all_handles containing all handles even the burned ones. In create profile check that vars.handle is non existent on all_handles

oneski commented 2 years ago

declining. this is as designed. when a user burns profile it re-enters the pool of available profiles to mint.

0xleastwood commented 2 years ago

I'd like to confirm a couple things on this issue. If User B has taken over the recently deleted profile, do they claim the followers User A previously had or are they expected to have people follow them again. If it is the latter, I'd say this is pretty consistent with how centralized platforms operate. If you delete your profile, you forfeit your username which can be claimed by any other user.

0xleastwood commented 2 years ago

I'm interested in hearing your thoughts on this @oneski

Zer0dot commented 2 years ago

No they should not claim anything User A previously had, they should start absolutely from scratch. There was a bug found by some wardens where deleting would not completely clear everything as intended so if a new profile was created with the same handle, one could follow the previously deleted profile. This was due to the check only checking for zero equality and reverting but this has since been fixed as we check for the expected profile ID instead.

0xleastwood commented 2 years ago

Okay, I'll side with the sponsor on this because this is intended behaviour.