In `LensBaseERC721.sol#_transfer()` Lack of check `from==to`

[code423n4]

Lines of code

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

Vulnerability details

Impact

• In LensBaseERC721.sol#_transfer() balances of from and to are stored in temporary variables and after the subtration, addition operation the results are written to storage.
• In the second operation since the original balance is used in the addition operation, if from == to the address will have extra tokens in the balance.
• So a user can self transfer tokens for id, burn and transfer more than supplied assets from a pair contract

Proof of Concept

In LensBaseERC721.sol#_transfer() If from == to, the self transfer will double the _balances.

    function _transfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual {
        if (ownerOf(tokenId) != from) {
            revert Errors.InvalidOwner();
        }
        if (to == address(0)) {
            revert Errors.InvalidParameter();
        }

        _beforeTokenTransfer(from, to, tokenId);

        // Clear approvals from the previous owner
        _approve(address(0), tokenId);

        unchecked {
            --_balances[from];
            ++_balances[to]; // @audit from==to
        }
        _tokenData[tokenId].owner = to;

        emit Transfer(from, to, tokenId);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

In LensBaseERC721.sol#__transfer() add check:

    if (from == to) revert();

Assessed type

Other

[141345]

no loss

at most QA

[c4-pre-sort]

141345 marked the issue as low quality report

[c4-pre-sort]

141345 marked the issue as remove high or low quality report

[c4-sponsor]

donosonaumczuk marked the issue as sponsor disputed

[donosonaumczuk]

Balance remain the same:

      unchecked {
            --_balances[from]; // Decreases...
            ++_balances[to]; // ...and Increases :)
        }

Nothing from the stated is valid. I wouldn't even mark this as QA, not even OpenZeppelin (standard from the industry in terms of security) is checking for it, so self-transfers are allowed.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

I do agree with @donosonaumczuk on this one