search

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

0 stars 0 forks source link

Use of _mint instead of safeMint in function createProfile of LensHub.sol #69

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/core/LensHub.sol#L142-L149

Vulnerability details

The ERC721 standard states that the onERC721Received callback must be called when a mint or transfer operation occurs.

In case of Smart Contracts interacting as users of the Lens protocol during profile creation, it will not be notified with the onERC721Received callback, as expected according to the ERC721 standard.

This is due to wrong implementation using _mint in createProfile function.

Impact

The Profile NFT tokens will be trapped, in case a Smart Contract which correctly implements the onERC71Received callback, interacts with Lens protocol for Profile Creation. For non-contract addresses or EOA, this is a non-issue.

Proof of Concept

Contract : LensHub.sol Line 149 : 

function createProfile(DataTypes.CreateProfileData calldata vars)
    external
    override
    whenNotPaused
    onlyWhitelistedProfileCreator
{
    uint256 profileId = ++_profileCounter;
    _mint(vars.to, profileId);
...

}

Recommended Mitigation Steps

Use _safeMint instead of _mint in the createProfile function.

Zer0dot commented 2 years ago

Invalid. SafeMint is not safe and interacting contracts should be aware we don't use it.

0xleastwood commented 2 years ago

I'm not sure why this is classified as high risk. At best, it somewhat impacts availability of the protocol, but as the sponsor has mentioned, contracts interacting with the protocol should be aware that they do not perform onERC721Received callbacks. This will not impact EOAs at all and only prevent certain contracts from interacting with the protocol.

0xleastwood commented 2 years ago

Normally I'd mark this as medium risk because of how it impacts protocol availability but I don't think that applies to this issue. The profile creator must be whitelisted by the governance contract. It doesn't seem likely that the governance would willingly whitelist an incompatible contract. Worse case scenario is that they whitelist a contract that is compatible instead.