Art Gobblers contest - obront's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L521 https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L562 https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L584

Vulnerability details

Impact

If the Chainlink VRF V1 is sunset before the randProvider is updated, the protocol could get stuck in a permanent waitingForSeed state, bricking the entire project.

Proof of Concept

In the code comments, randProvider is commented with the following note:

/// a wrapper around Chainlink VRF v1, but can be changed in case it is fully sunset.

Similarly, the updateRandProvider function has the following comment:

/// @notice Upgrade the rand provider contract. Useful if current VRF is sunset.

These comments point to the fact that the Art Gobblers team sees the possibility that the Chainlink VRF V1 oracle may be sunset, and would like to be prepared for such a situation.

However, in the event that this sunsetting happens before randProvider is updated, the protocol will become irreversibly bricked.

• calling requestRandomSeed() will set waitingForSeed to true
• the only way to set it back to false would be for acceptRandomSeed() to be called, but this call will not be returned from the deprecated oracle
• we will not be able to call requestRandomSeed() again, because gobblerRevealsData.toBeRevealed will still be greater than zero
• the only way to set gobblerRevealsData.toBeRevealed back to 0 is to call revealGobblers(), which isn't allowed while waitingForSeed is true
• we are not able to update the oracle because upgradeRandProvider() also reverts on waitingForSeed

Thus, we are stuck in a loop where we are no longer allowed to request random seeds, are not able to accept random seeds, and therefore are not allowed to reveal any Gobblers.

Although this situation may be unlikely, the fact that the team is preparing for the possibility of Chainlink VRF V1 being sunset and that it could irreversibly take down the whole project leads me to a judge the severity as a high.

Tools Used

Manual Review

Recommended Mitigation Steps

Turn waitingForSeedinto a timestamp and have the requirement expire after some period of time.

Lines of code

dhttps://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L401 https://github.com/transmissions11/VRGDAs/blob/f4dec0611641e344339b2c78e5f733bba3b532e0/src/VRGDA.sol#L46-L50 https://github.com/transmissions11/VRGDAs/blob/f4dec0611641e344339b2c78e5f733bba3b532e0/src/LogisticVRGDA.sol#L62

Vulnerability details

Impact

In a situation where the demand pressure on the Gobblers gets too strong (366 days ahead), the auction will not behave as expected (continuously scaling up price to bring down demand), but will instead revert.

Proof of Concept

The price of a Gobbler is calculated by calling getVRGDAPrice() with days since start and the total number minted from goo as the arguments. The VRGDA library implements the following calculation:

uint256(
    wadMul(
        targetPrice, 
        wadExp(
            unsafeWadMul(
                decayConstant,
                timeSinceStart - getTargetSaleTime(toWadUnsafe(sold + 1))
            )
        )
    )
);

Let's focus in on the situation where the target sale time is at least 366 days in the future.

• timeSinceStart - getTargetSaleTime() will equal -366.
• with the decay constant set to ln(1e18 - 0.31e18) = -371063681390831986, their product will be 1.358e20
• when this is run through wadExp, it will revert on the check for if (x >= 135305999368893231589) revert("EXP_OVERFLOW");

The result is that the auction will revert, and sales will not be allowed until the price falls back within a 1 year range.

This is unlikely to be a problem early on in the auction, as there are thousands of Gobblers for sale per year. But towards the later years, this number falls all the way down to the single digits per year, so it is highly possible that high demand pushes the bid up enough to cause this error.

This undercuts the supposed mechanism of VRGDA, as the auction will not be able to scale up with demand and instead will turn the the buying process into a competition to be the first to get in when it falls back within range.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Fork SignedWadMath.sol and adjust it to allow for higher values, or make the necessary adjustments to store the price decay constant with fewer decimal places.