wise-foundation / lending-audit

5 stars 4 forks source link

[PNF-03M] Potentially Insecure Position Creation Workflow #153

Open vm06007 opened 1 year ago

vm06007 commented 1 year ago

PNF-03M: Potentially Insecure Position Creation Workflow

Type Severity Location
Logical Fault PositionNFTs.sol:L117-L126

Description:

The PositionNFTs::mintPositionForUser function can be invoked by anyone, causing the creation of an EIP-721 PositionNFTs entry to the target _user as well as a re-entrant call to them.

Impact:

Certain smart contracts may rely on a step-by-step position reservation and creation workflow both of which can have unexpected consequences (i.e. 2 position NFTs minted instead of one or a position NFT with an ID not equal to the reserved one).

Example:

/**
 * @dev Mints NFT for _user, without it
 * user can not use WiseLending protocol
 */
function mintPositionForUser(
    address _user
)
    external
    returns (uint256)
{
    return _mintPositionForUser(
        _user
    );
}

function _mintPositionForUser(
    address _user
)
    internal
    returns (uint256)
{
    uint256 nftId = reserved[
        _user
    ];

    if (nftId > 0) {
        delete reserved[
            _user
        ];

        totalReserved--;

    } else {
        nftId = getNextExpectedId();
    }

    _safeMint(
        _user,
        nftId
    );

    return nftId;
}

Recommendation:

We advise the code to either entirely omit the PositionNFTs::mintPositionForUser function or to ensure that the caller is properly authorized by the _user, f.e. by evaluating whether they are "approved-for-all" (ERC721::isApprovedForAll).

We consider adequate of the two solutions as sufficient in alleviation this exhibit.

vm06007 commented 1 year ago

Resolved in: https://github.com/wise-foundation/lending-audit/pull/152