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

Wrong name of contract in RamNFT

Summary

In the RamNFT contract there is an potential logic problem in that the contract's naming assumes that the ChoosingRam contract mints the NFTs. It is actually the Dussehra contract that does the minting, and the restrictions need to restrict to this contract instead. The modifier is ultimately not used anyway, but it is the wrong contract to begin with. The Dussehra.t.sol setUp() function sets the wrong contract for the whole test suite.

Vulnerability Details

The ability to update the contract address and the fact that this is stored as an address means that we can actually get the right functionality by updating the choosingRamContract with the address of the Dussehra contract, but this is confusing and the naming should be changed.

Once the modifier has been appended to the mint function in RamNFT:

+ function mintRamNFT(address to) public onlyChoosingRamContract {
- function mintRamNFT(address to) public {

then we can see from the current test suite that the wrong contract is used in the setUp() function. Leaving this we can prove that that fails and that using the Dussehra contract works correctly.

function test_enterPeopleWhoLikeRamContractCheck() public {
vm.deal(player1, 1 ether);
vm.startPrank(player1);
vm.expectRevert(RamNFT.RamNFT__NotChoosingRamContract.selector);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.prank(organiser);
ramNFT.setChoosingRamContract(address(dussehra));
vm.startPrank(player1);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
assertEq(dussehra.peopleLikeRam(player1), true);
assertEq(dussehra.WantToBeLikeRam(0), player1);
assertEq(ramNFT.ownerOf(0), player1);
assertEq(ramNFT.getCharacteristics(0).ram, player1);
assertEq(ramNFT.getNextTokenId(), 1);
}

Impact

No current impact, because even if the incorrect contract address is used, the modifier is not used, but if it was, then the whole NFT minting would fail and the protocol would be completely broken, as no-one could pay their fees and register their interest in Ram until the address was updated with the right Dussehra contract.

Tools Used

foundry + manual review.

Recommendations

Change the name of the variables, modifiers and error to accurately reflect the contract it refers to. Once done, change the Dussehra.t.sol setUp() function:

+ ramNFT.setChoosingRamContract(address(dussehra));
- ramNFT.setChoosingRamContract(address(choosingRam));
Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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