Fractionalized asset can be locked in vault forever.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L173-L202 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L398-L399

Vulnerability details

Impact

minBuyoutTime can be set to a value that block.timestamp will never reach. This would prevent the function initiateBuyout() from being called ever. Thus resulting in a forever fractionalized asset.

Proof of Concept

NibblVault.sol::initialize()

function initialize(
        string memory _tokenName, 
        string memory _tokenSymbol, 
        address _assetAddress,
        uint256 _assetID,
        address _curator,
        uint256 _initialTokenSupply,
        uint256 _initialTokenPrice,
        uint256 _minBuyoutTime
    ) external override initializer payable {
        ...
        minBuyoutTime = _minBuyoutTime; // @audit this can be set to 2^256 - 1
        ...
}

NibblVault.sol::initiateBuyout()

function initiateBuyout() external override payable whenNotPaused returns(uint256 _buyoutBid) {
        require(block.timestamp >= minBuyoutTime, "NibblVault: minBuyoutTime < now");
        // @audit block.timestamp would never be >= minBuyoutTime if set to a high value.

Tools Used

Manual.

Recommended Mitigation Steps

Consider adding a maximum threshold when setting minBuyoutTime.

[mundhrakeshav]

Desired Behaviour

[HardlyDifficult]

This is not a vulnerability since anyone can see this before interacting with the vault and the valuation of tokens should reflect this. Setting a max in the contract requires selecting an arbitrary value and it may be more appropriate for the frontend to suggest a limit here. This is a fair concern and potential consideration for the documentation and any integrators. Lowering the severity and merging with the warden's QA report #297