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

1 stars 1 forks source link

`NFTVault.sol::overrideFloor()` should have upper and lower bounds set. #181

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#L275-L281

Vulnerability details

Impact

There is nothing stopping DAO_ROLE from setting _newFloor to a really high/low value with malicious intent.

Proof of Concept

/// @notice Allows the DAO to bypass the floor oracle and override the NFT floor value
/// @param _newFloor The new floor
function overrideFloor(uint256 _newFloor) external onlyRole(DAO_ROLE) {
    require(_newFloor > 0, "Invalid floor");
    nftTypeValueETH[bytes32(0)] = _newFloor;
    daoFloorOverride = true;
}

Recommendation

Consider adding upper and lower bounds relative to the current NFT floor price when setting _newFloor.

Tools Used

Manual.

spaghettieth commented 2 years ago

We can't set any lower or upper bound as future market value may surpass any upper bound or go below the lower bound.

dmvt commented 2 years ago

Invalid and out of scope