code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

[WP-M17] `pendingNFTValueETH` should have an expiration time #161

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L347-L354

Vulnerability details

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L347-L354

function setPendingNFTValueETH(uint256 _nftIndex, uint256 _amountETH)
    external
    validNFTIndex(_nftIndex)
    onlyRole(DAO_ROLE)
{
    pendingNFTValueETH[_nftIndex] = _amountETH;
}

Due to the volatility of the NFT market, the price assessment made 1 month ago may not be accurate anymore 1 month later.

Therefore, using a stale pendingNFTValueETH can be harmful as it may lead to undercollateralized loans or bad debt to the protocol.

Recommendation

Consider adding an expiration time for every pendingNFTValueETH, and finalizePendingNFTValueETH can only be done before the expiration time.

spaghettieth commented 2 years ago

In case of a stale pending NFT value the DAO can just remove it by setting it to 0

dmvt commented 2 years ago

Out of scope