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

The function `ChoosingRam::increaseValuesOfParticipants` does not set the variable `isRamSelected`, meaning players cannot become Ram as described in the documentation.

poolig4n

Description
The function ChoosingRam::increaseValuesOfParticipants does not set isRamSelected when the variable selectedRam is set, resulting in no possible way players can use this function to become Ram for the event, which contradicts the documentation.

The documentation states (emphasis mine):

  1. increaseValuesOfParticipants allows users to increase their values(or characteristics) and become Ram for the event. The values will never be updated again after 12 October 2024.

  2. selectRamIfNotSelected - Allows the organizer to select Ram if not selected by the user.

Impact
The function increaseValuesOfParticipants is supposed to give players a chance to increase their values and potentially become Ram, but this function can only be called successfully if the modifier RamIsNotSelected returns true. The problem is that the modifier checks the boolean value of isRamSelected, but the function increaseValuesOfParticipants does not set the isRamSelected boolean value, so the players can never "become Ram" using this function.

Evidence
The modifier used on increaseValuesOfParticipants:

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

The function signature of increaseValuesOfParticipants:

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger,
uint256 tokenIdOfAnyPerticipent)
public
modifier *-----> RamIsNotSelected {
// function body ommitted for readability.

The relevant part of the function increaseValuesOfParticipants is shown below. This function does not currently set isRamSelected=true when it sets the selectedRam variable. The diff indicates one possible way to address the finding:

selectedRam = ramNFT.getCharacteristics(tokenIdOfChallenger).ram;
+ isRamSelected = true;
​
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;
+ isRamSelected = true;
​

Please note this should not be considered production ready code. This is indicative of one possible solution and should be considered within the context of the protocol as to the suitability of this as a solution.

Proof of Concept

Standalone test

// 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 { mock } from "../src/mocks/mock.sol";
import {RamNFT} from "../src/RamNFT.sol";
​
contract IncreaseValueTest is Test {
​
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
address public organiser = makeAddr("organiser");
address public player1 = makeAddr("player1");
​
modifier participants() {
vm.startPrank(player1);
vm.deal(player1, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
_;
}
function setUp() public {
vm.startPrank(organiser);
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(1 ether, address(choosingRam), address(ramNFT));
​
ramNFT.setChoosingRamContract(address(choosingRam));
vm.stopPrank();
}
​
// Audit tests
​
function test_increaseValuesOfParticipantsToSelectRam() public participants {
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
vm.stopPrank();
assertEq(ramNFT.getCharacteristics(1).isJitaKrodhah, true);
assertEq(ramNFT.getCharacteristics(1).isDhyutimaan, true);
assertEq(ramNFT.getCharacteristics(1).isVidvaan, true);
assertEq(ramNFT.getCharacteristics(1).isAatmavan, true);
assertEq(ramNFT.getCharacteristics(1).isSatyavaakyah, true);
​
//selectedRam is set
assertEq(choosingRam.selectedRam(), ramNFT.getCharacteristics(1).ram );
​
// isRamSelected is not set to true!
// This should be true, not false, when selectedRam is set.
assertEq(choosingRam.isRamSelected(), false);
}
​

Test results

forge test --match-path test/IncreaseValueTest.t.sol -vv
[⠊] Compiling...
[⠢] Compiling 1 files with 0.8.20
[⠆] Solc 0.8.20 finished in 4.25s
Compiler run successful!
​
Ran 1 test for test/IncreaseValueTest.t.sol:IncreaseValueTest
[PASS] test_increaseValuesOfParticipantsToSelectRam() (gas: 313572)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.92ms (957.00µs CPU time)
​
Ran 1 test suite in 472.00ms (2.92ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

References

Tools Used

  • Manual Review

Updates

Lead Judging Commences

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

`isRamSelected` is not set

Support

FAQs

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