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: 5/96
Findings: 1
Award: $2,339.34
🌟 Selected for report: 1
🚀 Solo Findings: 1
🌟 Selected for report: itsmeSTYJ
2339.3444 USDC - $2,339.34
https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/NibblVault.sol#L195
There is no sanity check to ensure that (primaryReserveRatio * _initialTokenSupply * _initialTokenPrice)
is ≥ (SCALE * 1e18)
. As a result, _primaryReserveBalance
is given a value of 0 since divisions in solidity are rounded down. This also means that primaryReserveBalance
and fictitiousPrimaryReserveBalance
have a value of 0.
When this happens, the buy
function doesn’t work because _chargeFee
will revert on line 222. You can’t sell
either since totalSupply == initialSupply
. The only way to recover the NFT is for the owner of the NFT to call initiateBuyout
but there is always the possibility that someone else also spotted this mistake and will attempt to also call initiateBuyout
once the minBuyoutTime
is reached. If the owner loses this gas war, the owner has effectively lost his NFT.
Add some sanity checks to ensure a sane expected value for _initialTokenSupply
and _initialTokenPrice
. There were multiple instances when a user tried to interact with a contract but entered a wrong value because they are not aware they needed to include decimals. A recent example of this is https://cointelegraph.com/news/1-million-rock-nft-sells-for-a-penny-in-all-ore-nothing-error.
Note: Normally, I would categorise issues like this as medium severity since it is a loss predicated on having met certain conditions but because there is also a lack of sanity check on _minBuyoutTime
, it is entirely possible for a seller to lose his NFT immediately once the vault is created. There are many monsters waiting in the dark forest, all it takes is one mistake. That said, I will defer the final judgement to the judges & sponsors.
#0 - mundhrakeshav
2022-06-25T14:53:07Z
Case when that would happen is _initialTokenSupply or _initialTokenPrice is 0. But then it would revert here https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L183
#1 - HardlyDifficult
2022-07-02T23:05:55Z
Case when that would happen is _initialTokenSupply or _initialTokenPrice is 0.
primaryReserveRatio = 200_000 and SCALE = 1_000_000 -- so it seems this applies anytime _initialTokenSupply * _initialTokenPrice < 1_000_000 * 1e18 / 200_000, not just when one of those values is 0. Please correct me if I got that wrong.
#2 - HardlyDifficult
2022-07-02T23:21:59Z
This is a good report. I agree this seems like something that should be addressed. Assuming my math is right, if the initialTokenPrice was 1 wei then the initialTokenSupply must be >= 5e18. They are using the default of 18 decimals (which is also industry standard) so that's 5 tokens. Given this is not very large window and these values impact the curve -- without a clear POC High would not be warranted, downgrading to Medium risk.