Aave Lens contest - danb's results

Lines of code

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

Vulnerability details

When someone tries to follow a profile, it checks if the handle exists, and if it doesn't, it reverts because the profile is deleted. The problem is that there might be a new profile with the same handle as the deleted one, allowing following deleted profiles.

Proof of Concept

Alice creates a profile with the handle "alice." The profile id is 1. she deleted the profile. she opens a new profile with the handle "alice". The new profile id is 2. bob tries to follow the deleted profile (id is 1). the check

if (_profileIdByHandleHash[keccak256(bytes(handle))] == 0)
	revert Errors.TokenDoesNotExist();

doesn't revert because there exists a profile with the handle "alice". Therefore bob followed a deleted profile when he meant to follow the new profile.

Recommended Mitigation Steps

change to:

if (_profileIdByHandleHash[keccak256(bytes(handle))] != profileIds[i])
	revert Errors.TokenDoesNotExist();

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/LensHub.sol#L929

Vulnerability details

All the external function of LensHub have whenNotPasued modifier. However, LensHub is erc721 and the transfer function doesn't have the whenNotPaused modifier.

Impact

In case where the governance wants to stop all activity, they still can't stop transferring profiles nfts. an example where stopping transferring tokens was actually very helpful: https://mobile.twitter.com/flashfish0x/status/1466369783016869892

Recommended Mitigation Steps

add whenNotPasued to _beforeTokenTransfer