code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/base/LensBaseERC721.sol#L218 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/base/LensBaseERC721.sol#L245 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/base/LensBaseERC721.sol#L264

Vulnerability details

Impact

The LensBaseERC721.transferFrom/safeTransferFrom/burn they check approvals on msg.sender through _isApprovedOrOwner(msg.sender, tokenId), it is never checked that the specified from parameter is actually the owner of the NFT.

An attacker can decrease other users' NFT balances locking users' funds. The attacker transfers their own NFT passing the victim as from by calling transfer- From(from=victim, to=attackerAccount, tokenId=attackerTokenId).

This passes the _isApprovedOrOwner check, but reduces from's balance.

similar to this one

Tools Used

Manual Review

Recommended Mitigation Steps

Add the following check to

require(ownerOf(tokenId) == from, Errors.ACCESS);

Assessed type

ERC721

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

invalid

Picodes commented 1 year ago

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/base/LensBaseERC721.sol#L413

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-07-lens-findings