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

Everyone can mint `RamNFT` without paying entrance fee and even become Ram with it

NightHawK

Summary

According to the documentation only the Dussehra contract is supposed to mint RamNFTs and only after payment of the entrance fee. However, this logic is not implemented in the contract and everyone can mint RamNFT without payment. Furthermore, they can use this minted NFT into challenges and become the Ram.

Vulnerability Details

RamNFTs are minted by the RamNFT::mintRamNFT function. This function takes the adress to which the NFT is minted. It does not check whether it is executed from the deployed Dussehra contract or not. This breaks the described logic. Furthermore, due to another vulnerability the users can challenge the same or different minted to them NFTs and be sure to become the Ram no matter the implemented random logic.

Proof of Concept

The following test demonstrates that everyone can mint RamNFT without payment of the entrance fee.

function test_userCanMintNFT() public {
Dussehra dussehra;
RamNFT ramNFT;
ChoosingRam choosingRam;
address organiser = makeAddr("organiser");
address player1 = makeAddr("player1");
​
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();
​
vm.startPrank(player1);
// the user who is not in the contest executes mintRamNFT function
ramNFT.mintRamNFT(player1);
vm.stopPrank();
​
// the NFT is minted successfully
assertEq(ramNFT.ownerOf(0), player1);
}

The next code demonstrates how the users can even mint multiple NFTs which can further be used so that the user became Ram.

function test_userCanMintMultipleNFTs() public {
Dussehra dussehra;
RamNFT ramNFT;
ChoosingRam choosingRam;
address organiser = makeAddr("organiser");
address player1 = makeAddr("player1");
​
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();
​
vm.startPrank(player1);
vm.deal(player1, 1 ether);
// the user enters the contest
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
// after this the user will have a legit RamNFT
​
// but he can also mint another RamNFT
ramNFT.mintRamNFT(player1);
vm.stopPrank();
​
// we can see that both RamNFTs belong to the same user
assertEq(ramNFT.ownerOf(0), ramNFT.ownerOf(1));
}

Impact

The missing check for who can execute RamNFT::mintRamNFT breaks the logic of the contract. Further, due to another vulnerability with a different root cause (the ChoosingRam::increaseValuesOfParticipants function does not check if the challenged user is the same as the challenger) the users will be able to challenge themselves and easily to become Ram. Even if the other vulnerability is fixed, the reported in this finding is still valid. It allows user who has not paid the entrance fee to become Ram and to get the Ram reward.

Tools Used

Manual review

Recommendations

Add checks for who is allowed to execute RamNFT::mintRamNFT in a similar way to the one for RamNFT::updateCharacteristics but this time for the Dussehra contract. See the code below.

contract RamNFT is ERC721URIStorage {
...
​
+ error RamNFT__NotDussehraContract();
​
+ modifier onlyDussehraContract() {
+ if (msg.sender != dussehraContract) {
+ revert RamNFT__NotDussehraContract();
+ }
+ _;
+ }
​
+ address public dussehraContract;
​
+ function setDussehraContract(
+ address _dussehraContract
+ ) public onlyOrganiser {
+ dussehraContract = _dussehraContract;
+ }
​
...
​
- function mintRamNFT(address to) public {
+ function mintRamNFT(address to) public onlyDussehraContract {
...
}
​
...
}

Then you should use ramNFT.setDussehraContract(address(dussehra)); to set the address of the deployed Dussehra contract such like using the ramNFT.setChoosingRamContract(address(choosingRam)); to set the address of the ChoosingRam contract.

The code below is a test which proves that with this changes no one other than the Dussehra contract can mint RamNFT.

error RamNFT__NotDussehraContract();
​
function test_userCannotMintNFT() public {
Dussehra dussehra;
RamNFT ramNFT;
ChoosingRam choosingRam;
address organiser = makeAddr("organiser");
address player1 = makeAddr("player1");
​
vm.startPrank(organiser);
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(1 ether, address(choosingRam), address(ramNFT));
ramNFT.setChoosingRamContract(address(choosingRam));
​
// we set the address of the Dussehra contract
ramNFT.setDussehraContract(address(dussehra));
vm.stopPrank();
​
// we expect a revert with RamNFT__NotDussehraContract
vm.expectRevert(
abi.encodeWithSelector(RamNFT__NotDussehraContract.selector)
);
​
vm.startPrank(player1);
// the user tries to mint RamNFT which will revert
ramNFT.mintRamNFT(player1);
vm.stopPrank();
}
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.