Eggstravaganza

First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Poor randomness in EggHuntGame::searchForEgg() leads to predictable outcomes, and lowers the eggFindThreshold

Summary

EggHuntGame::searchForEgg() relies on the following code to generate a pseudorandom number between 0 and 99:

uint256 random = uint256(
keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender, eggCounter))
) % 100;

Each call of the EggHuntGame::searchForEgg() function should have an independent, random result, i.e., calling the function multiple times should increase your chances of success. However, this is not the case. As seen above, the randomness relies on 4 variables: block.timestamp, block.prevrandao, msg.sender,and eggCounter. In case of success, the eggCounter is increased by 1. The result of a successful search will be different from the next time the function is called. However, in case of a failed search, multiple function calls return the same random number when called in the same block i.e., the probability of a successful search will be lower than the default eggFindThreshold.

Impact

Poor randomness leads to reduced chances of success for the users playing fairly.

Proof of Concept

Copy the following into the test file and run with forge test --mt testRandomness.

function testRandomness() public {
uint256 duration = 200;
game.startGame(duration);
vm.startPrank(alice);
for(uint256 i = 0; i < 150; i++) {
game.searchForEgg();
}
assertEq(game.eggCounter(), 0); // success rate = 0
vm.warp(5); // different warp times give different results.
// some cases increase eggCounter a few times before getting stuck with the failure result for the rest of the loop.
for(uint256 i = 0; i < 150; i++) {
game.searchForEgg();
}
vm.stopPrank();
assertEq(game.eggCounter(), 0); // success rate = 0
}

Expected result:

[⠢] Compiling...
[⠊] Compiling 1 files with Solc 0.8.28
[⠒] Solc 0.8.28 finished in 668.91ms
Compiler run successful with warnings:
Warning (2018): Function state mutability can be restricted to view
--> test/EggHuntGameTest.t.sol:232:5:
|
232 | function testGameStatusBeforeStart() public {
| ^ (Relevant source part starts here and spans across multiple lines).
Ran 1 test for test/EggHuntGameTest.t.sol:EggGameTest
[PASS] testRandomness() (gas: 581198)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.21ms (865.22µs CPU time)
Ran 1 test suite in 3.34ms (1.21ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools

Manual Review, Slither

Recommendation

An external source of randomness would be the best solution in this case. However, at a minimum, using a nonce would prevent the same pseudorandom number being generated multiple times per block.

Updates

Lead Judging Commences

m3dython Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Insecure Randomness

Insecure methods to generate pseudo-random numbers

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.