Trick or Treat

First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Invalid

The probablity of user being `tricked` or `treated` is not 1/1000

Vulnerability Details

As trick or treat condition depends on the value random of trickOrTreat function

uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender, nextTokenId, block.prevrandao))) % 1000 + 1;

I wrote a fuzz test trying to simulate the calculation of random and checking the probability of getting 1 or 2.

// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v5.1.0) (token/ERC721/ERC721.sol)
pragma solidity ^0.8.20;
import {Test} from "../lib/forge-std/src/Test.sol";
import {console} from "../lib/forge-std/src/console.sol";
import "../src/TrickOrTreat.sol";
contract testKeccack is Test {
SpookySwap spooky;
address owner = makeAddr("owner");
address user = makeAddr("user");
function setUp() public {
vm.startPrank(owner);
//spooky.Treat;
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
//treats[]
treats[0] = SpookySwap.Treat({name: "Candy", cost: 0.1 ether, metadataURI: "candy-uri"});
treats[1] = SpookySwap.Treat({name: "Chocolate", cost: 0.2 ether, metadataURI: "chocolate-uri"});
treats[2] = SpookySwap.Treat({name: "Lollipop", cost: 0.15 ether, metadataURI: "lollipop-uri"});
spooky = new SpookySwap(treats);
vm.stopPrank();
vm.deal(user, 100 ether);
}
function test_RandomDistribution() public {
uint256 runs = 1000;
uint256 ones = 0;
uint256 twos = 0;
for (uint256 i = 0; i < runs; i++) {
// Generate different inputs for each run
uint256 timestamp = i + block.timestamp;
uint256 prevrandao = i + block.prevrandao;
address sender = address(uint160(i + 1));
uint256 nextTokenId = 1;
uint256 random = uint256(keccak256(abi.encodePacked(timestamp, sender, nextTokenId, prevrandao))) % 1000 + 1;
if (random == 1) ones++;
if (random == 2) twos++;
}
console.log("Distribution over", runs, "runs:");
console.log("Ones:", ones);
console.log("Twos:", twos);
// Log the actual probabilities
console.log("Probability of getting 1:", (ones * 100) / runs, "%");
console.log("Probability of getting 2:", (twos * 100) / runs, "%");
// Optional: Add assertions to verify the distribution is roughly as expected
// Note: Due to randomness, these might occasionally fail
assertLt(ones, runs / 500); // Should be roughly 0.1% (1/1000)
assertLt(twos, runs / 500); // Should be roughly 0.1% (1/1000)
}
}

When this test is run, it gives the following output

Ran 1 test for src/Fuzz.t.sol:testKeccack
[PASS] test_RandomDistribution() (gas: 802745)
Logs:
Distribution over 1000 runs:
Ones: 0
Twos: 0
Probability of getting 1: 0 %
Probability of getting 2: 0 %
Traces:
[802745] testKeccack::test_RandomDistribution()
├─ [0] console::log("Distribution over", 1000, "runs:") [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Ones:", 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Twos:", 0) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Probability of getting 1:", 0, "%") [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Probability of getting 2:", 0, "%") [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertLt(0, 2) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertLt(0, 2) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.26ms (5.02ms CPU time)

As we can see both probability of getting both 1,2 is zero. I've tried running this test multiple times and the answer remains same. But if we increase the number of runs greater than 1000, then there are situations where 1,2s appear.

Impact

Trick or Treat conditions never occur in 1000 runs.

Tools used

Manual review

Reccomended mitigation

Improve the random number generation technique.

Updates

Appeal created

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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