Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

uses a weak PRNG in `SpookySwap::trickOrTreat()`

aliqbdoli

Summary

The PRNG is highly predictable and can be manipulated by miners or attackers,Do not rely on block.timestamp or blockhash as a source of randomness .

Using oracle chainlink gives you a reliable and secure random source

Vulnerability Details

Using this code to generate random numbers is not correct and can be manipulated

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

Impact

Random numbers can be guessed in this way

Tools Used

personal knowledge , Slither

Recommendations

Using Chainlink VRF

Set up the oracle contract: Create a separate contract that interacts with the oracle service. Here's a simplified example

import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
​
contract RandomOracle is VRFConsumerBase {
bytes32 internal keyhash;
uint256 internal fee;
​
constructor(address _vrfCoordinator, address _link, bytes32 _keyhash, uint256 _fee)
VRFConsumerBase(_vrfCoordinator, _link)
{
keyhash = _keyhash;
fee = _fee;
}
​
function getRandomNumber() public payable returns (bytes32 requestId) {
return requestRandomness(keyhash, fee);
}
​
function fulfillRandomness(bytes32 requestId, uint256 randomness)
internal
{
// Store the random number here
// You can emit an event or perform actions based on this randomness
}
}
​

Implement the main contract: Modify your SpookySwap contract to use the RandomOracle:

import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
​
contract SpookySwap is ERC721URIStorage, VRFConsumerBase, Ownable {
RandomOracle public randomOracle;
uint256 public nextTokenId;
​
constructor(address _randomOracleAddress, address _vrfCoordinator, address _link, bytes32 _keyhash, uint256 _fee)
ERC721("SpookyTreats", "SPKY")
VRFConsumerBase(_vrfCoordinator, _link)
{
randomOracle = RandomOracle(_randomOracleAddress);
nextTokenId = 1;
}
​
function getRandomNumber() public payable returns (bytes32 requestId) {
return randomOracle.getRandomNumber();
}
​
// ... rest of your contract functions ...
​
function trickOrTreat(string memory _treatName) public payable {
bytes32 requestId = getRandomNumber();
​
// Wait for oracle response
// This is a simplified example - in practice, you'd need to wait for the callback
// and verify the request ID matches
​
// Simulating oracle response
uint256 random = 123; // Replace with actual oracle response
​
// Rest of your logic using the random number...
}
}
​
Updates

Appeal created

bube Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] Weak randomness

It's written in the README: "We're aware of the pseudorandom nature of the current implementation. This will be replaced with Chainlink VRF in later builds." This is a known issue.

Support

FAQs

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