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

`RamNFT::mintRamNFT` can be called by everybody.

kostov

Summary

Everybody can call RamNFT::mintRamNFT which breaks the contract invariant.

Vulnerability Details

There is no check if the msg.sender is the Dussehra contract, so every user can call RamNFT::mintRamNFT and mint as many nfts as they wish for their account.

function test_notOnlyDussehraCanMinNft() public participants {
ramNFT.mintRamNFT(player2);
uint256 cuurrentTokenId = ramNFT.getNextTokenId() - 1;
address ramOwner = ramNFT.getCharacteristics(cuurrentTokenId).ram;
​
vm.assertEq(player2, ramOwner);
}

Impact

Users can mint their NFT without paying a fee to Dussehra.sol contract. Moreover, they have a chance to become the Ram and withdraw their reward from the Dussehra.sol contract. Actually, their chances of becoming the Ram are increased, because they can mint 2 nfts and then keep calling ChoosingRam::increaseValuesOfParticipants with their two tokenIds. At the latest as of the 9th call(could happen and earlier) of the function one of their NFT will become the Ram.

In normal circumstances, if users keep calling ChoosingRam::increaseValuesOfParticipants they can make the participant's nft the Ram and that possibility does not exist if the two tokenIds are owned by the same user.

Tools Used

Unit testing

Manual Review

Recommendations

Add a modifier that checks if the msg.sender is the Dussehra contract and revert if it is not.

error RamNFT__NotChoosingRamContract();
+ error RamNFT_NotDussehraContract();
...
​
address public choosingRamContract;
+ address public dussehraContract;
​
...
​
+ modifier onlyDussehraContract() {
+ if (msg.sender != dussehraContract) {
+ revert RamNFT_NotDussehraContract();
+ }
+ _;
+ }
​
+ function mintRamNFT(address to) public onlyDussehraContract {
uint256 newTokenId = tokenCounter++;
_safeMint(to, newTokenId);
​
Characteristics[newTokenId] = CharacteristicsOfRam({
ram: to,
isJitaKrodhah: false,
isDhyutimaan: false,
isVidvaan: false,
isAatmavan: false,
isSatyavaakyah: false
});
}
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.