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

Anyone can mint NFTs for free due to missing access control in `RamNFT::mintRamNFT`

paprikrumplikas

Summary

RamNFT::mintRamNFT lacks access control and allows anyone to mint any number of NFTs for free.

Vulnerability Details

RamNFT::mintRamNFT is supposed to allow only the Dussehra contract to mint NFTs - this is to ensure that the protocol issues NFTs to only those users who call Dussehra::enterPeopleWhoLikeRam and pay the entrance fee for the event.

However, RamNFT::mintRamNFT fails to implement to neccessary access control. Consequently, users can mint NFTs for free by directly calling RamNFT::mintRamNFT.

The following test demonstrates thatplayer1 can mint an NFT for free in such a way:

function test_canMintNftForFree() public {
// player 1 calls RamNFT.mintRamNft directly to mint for free
vm.prank(player1);
ramNFT.mintRamNFT(player1);
​
// player1 has the NFT
assertEq(ramNFT.ownerOf(0), player1);
assertEq(ramNFT.getCharacteristics(0).ram, player1);
}

Impact

Users can mint NFTs for free and can participate in the event as if they paid the entrance fee.

Tools Used

Manual review, Foundry.

Recommendations

Implement the proper access control as follows:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {ERC721URIStorage, ERC721} from "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
​
contract RamNFT is ERC721URIStorage {
error RamNFT__NotOrganiser();
error RamNFT__NotChoosingRamContract();
+ error RamNFT__NotDussehraContract();
​
​
// https://medium.com/illumination/16-divine-qualities-of-lord-rama-24c326bd6048
struct CharacteristicsOfRam {
address ram;
bool isJitaKrodhah;
bool isDhyutimaan;
bool isVidvaan;
bool isAatmavan;
bool isSatyavaakyah;
}
​
uint256 public tokenCounter;
address public organiser;
address public choosingRamContract;
+ address dussehraAddress;
​
mapping(uint256 tokenId => CharacteristicsOfRam) public Characteristics;
​
modifier onlyOrganiser() {
if (msg.sender != organiser) {
revert RamNFT__NotOrganiser();
}
_;
}
​
modifier onlyChoosingRamContract() {
if (msg.sender != choosingRamContract) {
revert RamNFT__NotChoosingRamContract();
}
_;
}
​
constructor() ERC721("RamNFT", "RAM") {
tokenCounter = 0;
organiser = msg.sender;
}
​
+ function setDussehraAddress(address _dussehraAddress) public onlyOrganiser {
+ dussehraAddress = _dussehraAddress;
+ }
​
function setChoosingRamContract(address _choosingRamContract) public onlyOrganiser {
choosingRamContract = _choosingRamContract;
}
function mintRamNFT(address to) public {
+ if(msg.sender != dussehraAddress){
+ revert RamNFT__NotDussehraContract();
+ }
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.