[PNF-05M] EIP-721 Deviation of Approval

[vm06007]

PNF-05M: EIP-721 Deviation of Approval

Type	Severity	Location
Standard Conformity		PositionNFTs.sol:L167-L169

Description:

The PositionNFTs::approve function override introduces a special condition that checks whether the _nftId to be approved is 0 and simply returns in direct contradiction with the EIP-721 standard.

Impact:

A deviation from the EIP-721 standard is significant and can cause integrators of the PositionNFTs to fail fatally.

Example:

function approve(
    address _spender,
    uint256 _nftId
)
    public
    override(
        ERC721,
        IERC721
    )
{
    if (_nftId == 0) {
        return;
    }

    if (reserved[msg.sender] == _nftId) {
        approveNative(
            _spender,
            _mintPositionForUser(
                msg.sender
            )
        );

        return;
    }

    approveNative(
        _spender,
        _nftId
    );
}

Recommendation:

We advise the code to instead skip the reserved entry validation if the _nftId is 0, ensuring that the PositionNFTs::approveNative call is ultimately executed and fails for the 0 value ID.

[vm06007]

Resolved as part of https://github.com/wise-foundation/lending-audit/pull/113 and https://github.com/wise-foundation/lending-audit/pull/8

[vm06007]

there is no longer apporve() default overwrite instead custom function approveMint, the nftId is also now preminted for the FeeManager thus cannot be used and the check is safe. If check can be considered as validation/sanitize the method