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

1 stars 1 forks source link

[WP-M18] `nftValueETH` should expire after JPEG unlocked #163

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#L360-L381

Vulnerability details

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

function finalizePendingNFTValueETH(uint256 _nftIndex)
    external
    validNFTIndex(_nftIndex)
{
    uint256 pendingValue = pendingNFTValueETH[_nftIndex];
    require(pendingValue > 0, "no_pending_value");
    uint256 toLockJpeg = (((pendingValue *
        _ethPriceUSD() *
        settings.creditLimitRate.numerator) /
        settings.creditLimitRate.denominator) *
        settings.valueIncreaseLockRate.numerator) /
        settings.valueIncreaseLockRate.denominator /
        _jpegPriceUSD();

    //lock JPEG using JPEGLock
    jpegLocker.lockFor(msg.sender, _nftIndex, toLockJpeg);

    nftTypes[_nftIndex] = CUSTOM_NFT_HASH;
    nftValueETH[_nftIndex] = pendingValue;
    //clear pending value
    pendingNFTValueETH[_nftIndex] = 0;
}

When the nftValueETH is set by the DAO, a certain amount of JPEG tokens are required to be locked.

However, in the current implementation, when the locked JPEG tokens unlock, nftValueETH remains valid and the user can withdraw all the locked JPEG tokens without repaying the debt first.

If the market price for the NFT drops and in combination with unlocked JPEG tokens, making the user incline to default the loan. As a result, the risk of bad debt increases for the protocol.

Recommendation

Consider requiring a re-assessment of nftValueETH before the JPEG tokens are unlocked, or the nftValueETH from a year ago should be considered stale and not good to continue being used as the collateral, therefore the loan must be repaid before the user can retrieve the unlocked JPEG tokens.

spaghettieth commented 2 years ago

The new NFT value doesn't expire after tokens are unlocked by design.

dmvt commented 2 years ago

Feature, not bug