Platform: Code4rena
Start Date: 21/06/2022
Pot Size: $30,000 USDC
Total HM: 12
Participants: 96
Period: 3 days
Judge: HardlyDifficult
Total Solo HM: 5
Id: 140
League: ETH
Rank: 7/96
Findings: 4
Award: $1,328.64
🌟 Selected for report: 1
🚀 Solo Findings: 0
1052.705 USDC - $1,052.70
https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L300 https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L362 https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L464 https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L495
While buy() and sell() are only callable when the system is not paused, redeem() and withdrawERC721() are also callable when it is not. This means that the BUYOUT_DURATION is ignored in such cases and it is possible that users are not able to reject certain buyouts.
A user initiates a buyout via initiateBuyout(). Just afterwards, the system is stopped. The token holders now cannot buy new tokens to increase the value. However, after two days, the bidder can still withdraw the NFT, i.e. there was no way for the users to reject this buyout.
It should be possible to reset the buyoutEndTime (to the current block.timestamp) when the system is paused such that the token holders always have the possibility to reject a buyout.
#0 - mundhrakeshav
2022-06-25T16:39:24Z
Expected. When paused no operations should be available.
#1 - fatherGoose1
2022-06-27T00:00:30Z
Strongly disagree with the sponsor's comment. Given that redeem() and withdrawERC721() DO NOT contain the whenNotPaused modifier, this ensures that pauses that occur during a buyout process will ensure the success of the buyout. The buyout success occurs by time passing a certain block.timestamp and the functionality to claim the NFT and retrieve the underlying are left open even during the pause.
Similar to issue #261
I would agree with the sponsor if all of the withdraw()/redeem() functions contained the whenNotPaused modifier so that truly all functions were locked during a pause.
#2 - mundhrakeshav
2022-06-27T02:06:39Z
Hmmm. Makes sense. We should pause redeem and Withdraw too.
#3 - HardlyDifficult
2022-06-30T01:07:14Z
The readme does include "Out of scope: Admin can pause and change certain parameters of the contract." however this report is not strictly about the ability to pause.
It should be possible to reset the buyoutEndTime
In this scenario, an end time has already been defined. If pause is used at that time the window shortens or closes so when resumed the opportunity may have been missed already. The warden's recommendation here, or some variation of it, would provide a way to effectively allow the system to resume from where it left off when originally paused.
I suspect the alternative of also pausing redeem / withdraw is not sufficient, as the window to buy/sell will still potentially be passed by the time the system resumes.
I agree with the submitted Med risk for this issue since the "function of the protocol or its availability could be impacted".
230.2266 USDC - $230.23
When a buy can be filled only partially by the secondary curve and the rest is filled by the primary curve, no admin and curator fee is deducted for the part that is filled by the secondary curve (https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L319). Therefore, a user could avoid buying fees almost completely for certain orders (that increase the secondary curve just above the maximum value).
Also deduct a fee in the linked branch.
#0 - mundhrakeshav
2022-06-25T16:36:54Z
Expected
#1 - HardlyDifficult
2022-07-04T00:11:28Z
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0x52, 0xNazgul, 0xNineDec, 0xc0ffEE, 0xf15ers, 0xkatana, BowTiedWardens, Chom, ElKu, Funen, GalloDaSballo, JC, JMukesh, JohnSmith, Lambda, Limbooo, MadWookie, MiloTruck, Nethermind, Noah3o6, Nyamcil, Picodes, PwnedNoMore, Randyyy, RoiEvenHaim, SmartSek, StErMi, Tadashi, TerrierLover, TomJ, Tomio, Treasure-Seeker, UnusualTurtle, Varun_Verma, Wayne, Waze, _Adam, apostle0x01, asutorufos, berndartmueller, c3phas, catchup, cccz, cloudjunky, codexploder, cryptphi, defsec, delfin454000, dipp, ellahi, exd0tpy, fatherOfBlocks, hansfriese, hyh, joestakey, kebabsec, kenta, masterchief, minhquanym, naps62, oyc_109, pashov, peritoflores, reassor, rfa, robee, sach1r0, saian, sashik_eth, shenwilly, simon135, slywaters, sorrynotsorry, sseefried, unforgiven, xiaoming90, ych18, zuhaibmohd, zzzitron
28.4927 USDC - $28.49
block.timestamp % 2**32 (https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L445) will reset to 0 on Feb 07, 2106. This results in completely wrong values for the TWAV calculation (as it determines the seconds passed as 2**32 in such a case).buyoutValuationDeposit is not deleted in _rejectBuyout() (https://github.com/code-423n4/2022-06-nibbl/blob/71b639f977c0351c9928dd3b78eaa4bebb738bc1/contracts/NibblVault.sol#L424)#0 - HardlyDifficult
2022-07-04T00:18:28Z
#1 - HardlyDifficult
2022-07-04T17:49:22Z
Good reports, succinct format.
🌟 Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 8olidity, ACai, BowTiedWardens, Chandr, Chom, ElKu, Fitraldys, Funen, IgnacioB, JC, Lambda, Limbooo, MiloTruck, Noah3o6, Nyamcil, Picodes, Randyyy, SmartSek, StErMi, TerrierLover, TomJ, Tomio, UnusualTurtle, Waze, _Adam, ajtra, c3phas, cRat1st0s, catchup, codexploder, cryptphi, defsec, delfin454000, ellahi, exd0tpy, fatherOfBlocks, hansfriese, joestakey, kebabsec, kenta, m_Rassska, minhquanym, oyc_109, pashov, reassor, rfa, robee, sach1r0, saian, sashik_eth, simon135, slywaters, ych18, ynnad, zuhaibmohd
17.224 USDC - $17.22
unchecked to save gas:
_initialTokenSupply, meaning this value can always be used directly instead of doing the calculation.