Art Gobblers contest - zdhu's results

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L584 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#L553 https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/ArtGobblers.sol#L562

Vulnerability details

Impact

ArtGobblers contract expects that acceptRandomSeed must be called by the rand provider, and sets the flag gobblerRevealsData.waitingForSeed to fasle. But once the rand provider don't do that once, the flag makes both upgradeRandProvider and revealGobblers permanently unusable, it means no more gobblers can be revealed.

Despite the fact that Chainlink VRF V1 is a reputable service and unlikely to have this problem, it is advised to take the potential into account. Furthermore, as this project aims to be a decade-long self-sustaining ecosystem, a bigger risk may be introduced after the owner upgrades the rand provider.

Proof of Concept

• requestRandomSeed sets the flag to true.

// Prevent revealing while we wait for the seed.
gobblerRevealsData.waitingForSeed = true;

• Rand provider don't call acceptRandomSeed to unset the flag.
• revealGobblers is permanently wating for the flag.

// Can't reveal if we're still waiting for a new seed.
if (gobblerRevealsData.waitingForSeed) revert SeedPending();

• The owner can't upgradeRandProvider to resolve it.

// Revert if waiting for seed, so we don't interrupt requests in flight.
if (gobblerRevealsData.waitingForSeed) revert SeedPending();

• Can't call requestRandomSeed again to trigger the rand provider because toBeRevealed can only be reset to 0 in revealGobblers

// A random seed can only be requested when all gobblers from the previous seed have been revealed.
// This prevents a user from requesting additional randomness in hopes of a more favorable outcome.
if (gobblerRevealsData.toBeRevealed != 0) revert RevealsPending();

Tools Used

• VS Code
• Foundry

Recommended Mitigation Steps

As of the existing design, the owner can upgrade rand provider. Adding an function that can upgrade the rand provider and request the seed when pending may be a solution.

function upgradeRandProviderWhenSeedPending(RandProvider newRandProvider) external onlyOwner {
    if (!gobblerRevealsData.waitingForSeed) revert SeedNotPending(); // Only used when seed is pending.
    randProvider = newRandProvider; // Update the randomness provider.
    randProvider.requestRandomBytes(); // Request the seed and the rand provider will call acceptRandomSeed.

    emit RandProviderUpgraded(msg.sender, newRandProvider);
}

Even if the outdated rand provider finally calls acceptRandomSeed after the above function was called, the seed won't be unexpectedly updated due to the check.

if (msg.sender != address(randProvider)) revert NotRandProvider();