wise-foundation / lending-audit

5 stars 4 forks source link

[WSH-01M] Incorrect Validation of Reservation #9

Open vm06007 opened 1 year ago

vm06007 commented 1 year ago

WSH-01M: Incorrect Validation of Reservation

Type Severity Location
Logical Fault WiseSecurityHelper.sol:L922

Description:

The WiseSecurityHelper::checkOwnerPosition function will incorrectly validate whether the reserved NFT of the _caller is equal to the _nftId if it is 0. As the reserved mapping in PositionNFTs is pre-filled with 0 values, all callers who have not interacted with the contract have by-default "reserved" the 0 value _nftId.

Impact:

The 0 value NFT ID is presently "owned" by any user that has no reservations in the PositionNFTs contract indicating a significant flaw in relation to the relevant ID.

Example:

/**
 * @dev Helper function checking the owner
 * of {_nftId}. Is skipped when aaveHub or
 * power farm is calling the function.
 */
function checkOwnerPosition(
    uint256 _nftId,
    address _caller
)
    public
    view
{
    if (POSITION_NFTS.reserved(_caller) == _nftId) {
        return;
    }

    if (POSITION_NFTS.ownerOf(_nftId) == _caller) {
        return;
    }

    revert NotOwner();
}

Recommendation:

We advise the WiseSecurityHelper::checkOwnerPosition function to handle the _nftId as a special case when it is 0, ensuring that only the IERC721::ownerOf validation is performed as an NFT ID of 0 is valid per the configuration of DeclarationsFeeManager and specifically the FEE_MASTER_NFT_ID constant.

vm06007 commented 1 year ago

Issue resolved in https://github.com/wise-foundation/lending-audit/pull/8