Art Gobblers contest - 0x52's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L411-L471

Vulnerability details

Impact

The amount of gobblers owned by the address minting the legendary gobbler is incorrect, showing that the user owns 1 more gobbler than they do

Proof of Concept

getUserData[msg.sender].gobblersOwned = uint32(getUserData[msg.sender].gobblersOwned - cost + 1);

In ArtGobblers.sol#L458 the cost of the legendary gobbler is subtracted and 1 is added to account for the new legendary gobbler being minted.

_mint(msg.sender, gobblerId);

Later in L469 _mint is called which increments the number of gobblers owned again. This results in the number of gobbler stored for the user being incorrect. This causes GobblersERC721.sol#balanceOf to return the incorrect value and any contract or integration that makes use of this data will be given incorrect data.

Tools Used

Manual Review

Recommended Mitigation Steps

Since _mint already takes care of adding to owner count, L458 should be rewritten as follows:

getUserData[msg.sender].gobblersOwned = uint32(getUserData[msg.sender].gobblersOwned - cost);

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L396-L402

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

ArtGobblers.sol#L399-L401

uint256 timeSinceStart = block.timestamp - mintStart;

return getVRGDAPrice(toDaysWadUnsafe(timeSinceStart), numMintedFromGoo);

ArtGobblers.sol#gobblerPrice calculates time starting from mintStart to use when queuing price from VRGDA.

ArtGobblers.sol#L327

gobblerRevealsData.nextRevealTimestamp = uint64(_mintStart + 1 days);

The emissionMultiple for each gobbler isn't set until reveal which happens at a minimum of 1 day after mintStart (set in the constructor). For the day between minting and revealing, the VRGDA curve is effectively trying to mint the gobblers but no one has goo because the reveal hasn't assigned the emissionMultiples. The VRGDA curve is designed so that it targets a high number of gobblers that first day and the pricing is set to change the price of gobblers extremely aggressively. The combination of these 3 factors will make so that when users finally get goo, the prices of all the gobblers that should have been minted the first day will be next to nothing.

Tools Used

Manual Review

Recommended Mitigation Steps

L399 should be changed to make it so that gobblers can be bought until a minimum of 1 day after mintStart, to compensate for the reveal timing:

    uint256 timeSinceStart = block.timestamp - mintStart + 1 days;