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: 11/96
Findings: 3
Award: $1,098.20
🌟 Selected for report: 0
🚀 Solo Findings: 0
1052.705 USDC - $1,052.70
https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L300 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L362
Impacts are two fold:
admin
should not be allowed to stop users from selling/cashing out their assets under any circumstance.
Due to poor timing or malicious admin
activity, it could be the case that buy()
is put on pause right after initiateBuyout()
is called, making it impossible for a bid to be rejected.
modifier whenNotPaused() { require(!NibblVaultFactory(factory).paused(), 'NibblVault: Paused'); _; }
function buy(uint256 _minAmtOut, address _to) external override payable notBoughtOut lock whenNotPaused returns(uint256 _purchaseReturn) {
function sell(uint256 _amtIn, uint256 _minAmtOut, address payable _to) external override notBoughtOut whenNotPaused returns(uint256 _saleReturn) {
Manual Review
Remove whenNotPaused
from sell()
.
Prevent the pausing of buy()
if status = Status.buyout
.
#0 - KenzoAgada
2022-06-26T01:10:45Z
Contest readme: "Out of scope: Admin can pause and change certain parameters of the contract."
#1 - fatherGoose1
2022-06-26T10:27:12Z
Understood. Hoping that impact #2 listed above might lead to reconsideration of this design. If many (thousands?) of simultaneous markets are controlled by a single pause authority, its very likely that one or several NFTs will not receive a fair rejection process.
#2 - HardlyDifficult
2022-06-30T00:57:23Z
The report here could have used a better description, particularly if it's reported as a potential High severity. However 2) does touch on the same point as #55 - closing this as a dupe.
🌟 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.2806 USDC - $28.28
Setting the curator to the wrong address will result in permanent loss of functionality in the protocol. It is recommended to apply a two-step transfer.
safeTransfer()
can be used instead when sending from the current address.
Contract would need to be redeployed if the feeTo
variable is accidently set to address(0).
It is recommended to use safeTransfer() instead of transfer() to mitigate against ERC20 implementations that do not revert on failure.
Most constants defined in the contracts are fully capitalized, just this one example is not.
#0 - HardlyDifficult
2022-07-03T01:27:12Z
#1 - HardlyDifficult
2022-07-04T19:11:03Z
All good considerations
🌟 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.2238 USDC - $17.22
For loops can be optimized with unchecked incrementing of i
.
https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L506-L508 https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L43-L44
#0 - mundhrakeshav
2022-06-26T13:23:43Z
#15