MetadataRenderer: Block hash will provide poor randomness post-Merge

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L249-L252

Vulnerability details

The MetadataRenderer uses the concatenation of token ID and several block attributes to generate a pseudo-random seed. Although these block attributes can be manipulated by motivated adversaries, this is a common pattern for "good enough" pseudorandomness:

MetadataRenderer#_generateSeed

    /// @dev Generates a psuedo-random seed for a token id
    function _generateSeed(uint256 _tokenId) private view returns (uint256) {
        return uint256(keccak256(abi.encode(_tokenId, blockhash(block.number), block.coinbase, block.timestamp)));
    }

However, post-Merge, the pseudorandomness provided by blockhash will be much weaker, since it will no longer contain the output of proof-of-work hashing. Instead, the recommended source of onchain pseudorandomness is the prevRandao value from beacon chain state.

Post-Merge, the current DIFFICULTY opcode (0x44), will become PREVRANDAO and return the prevRandao value from the beacon chain. In current versions of Solidity, block.difficulty will return this value, since it uses opcode 0x44 under the hood.

See EIP-4399 and Solidity issue ethereum/solidity#13512 for more information on changes to DIFFICULTY and block.difficulty.

Impact

Random seeds may be more easily manipulable post-Merge, granting attackers control over which token attributes are generated at specific times.

Recommendation

Add block.difficulty as a component of the seed value:

    /// @dev Generates a psuedo-random seed for a token id
    function _generateSeed(uint256 _tokenId) private view returns (uint256) {
        return uint256(keccak256(abi.encode(_tokenId, block.difficulty, blockhash(block.number), block.coinbase, block.timestamp)));
    }

[GalloDaSballo]

Always was bad RNG

[eugenioclrc]

duplicate #151

[GalloDaSballo]

This looks like the best, making it primary

[horsefacts]

I want to point out #242 as another good finding here. Out of everyone who looked at this line and identified weak randomness, only 0xA5DF and Lambda noticed the biggest issue: blockhash(block.number) always returns zero and doesn't provide any randomness at all!

[GalloDaSballo]

My specific criticism against the PRNG findings being of Medium Severity is that in the context of this codebase, Randomness is more a tool for storytelling than a tool for scarcity.

Distribution of properties is linear, meaning that the PRNG is the way to create scarcity in features.

If any specific individual or group where to monitor the chain, they could all predictably decide when to settle an auction and trigger the next, meaning that a recognizably rare (where rarity is perceived or actualized), can be crated.

At that point the system would have a auction where most willing participant recognize the value of the new token, meaning that from "gaming" the system, the system is gaining.

If this were to be abused, to always mint the scarcest of trait, then over time the scarce trait would simply lose it's rarity

And because all features have a linear distribution, over enough time, no specific trait should is expected to emerge, although, per the above some trait may become rarer or scarcer.

All in all I think the rarity is secondary here, as no promise of fair distribution is made.

Because of that, I will confirm on a technical level that there are better ways to source scarcity (obvious one being VRF), however, because traits are secondary instead of being the main point of the collection (as in, there are no weights to determine if a feature should be scarce or common), I think Low Severity to be more appropriate.

In a different contest, where scarcity is necessary, (see Meebits for example), this finding may even be of High Severity.
However, in this codebase, where scarcity takes a backseat, I don't think gaming the system can do anything besides create more incentives for fund-raising, which actually helps the project instead of bringing it down

[GalloDaSballo]

L