search

code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

NibblVault buyout duration longer than update timelock #278

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Utilities/NibblVaultFactoryData.sol#L6 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L158-L169 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L37

Vulnerability details

Impact

User can buy out NFT by initiating the process through initiateBuyout, then he has to wait BUYOUT_DURATION which is 5 days and if the buyout will not get rejected he can claim the NFT. During that period bidder cannot cancel the process. The issue is that since NibblVault is used through proxy it is possible to change its implementation through administrative functionality in NibblVaultFactory and the timelock for update'ing implementation is only 2 days.

Attack Scenario:

  1. Bidder initiates buyout through initiateBuyout
  2. Administrator of the protocol updates the vaultImplementation through proposeNewVaultImplementation
  3. Bidder really does not like new implementation but cannot cancel buyout process
  4. Administrator waits 2 days (the UPDATE_TIME) uses updateVaultImplementation and changes the implementation
  5. Bidder loses funds/fait in the protocol

Proof of Concept

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to either implement functionality for bidder to cancel the bid or increase/decrease the UPDATE_TIME/BUYOUT_DURATION so the invariant BUYOUT_DURATION < UPDATE_TIME holds.

HardlyDifficult commented 2 years ago

It seems the bidder could be left in a bad state, and updating the thresholds here may be a nice way to maintain expectations. Since this scenario is based on the admin making an undesirable change, this is a Medium risk report.