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

The organizer can set the address of a `choosingRamContract` using `RamNFT::setChoosingRamContract`, leading to a potential denial-of-service (DoS) vulnerability.

v1vah0us3

Vulnerability Details

The setChoosingRamContract(address) function allows an event organizer to freely update the address of the choosingRamContract variable. The organizer could intentionally or unintentionally set a new address to a choosingRamContract variable by calling setChoosingRamContract(address). In this case, users who would call ChoosingRam::increaseValuesOfParticipants will not be able to update the properties of their NFTs. Due to the logic of ChoosingRam::increaseValuesOfParticipants it makes an external call to RamNFT::updateCharacteristics. However the access to RamNFT::updateCharacteristics is protected with an onlyChoosingRamContract modifier. If the organizer has updated the address of the choosingRamContract variable, the call to RamNFT::updateCharacteristics as well as to ChoosingRam::increaseValuesOfParticipants will revert, leading to a potential denial-of-service (DoS) vulnerability.

Impact

Organizer can update the address of the choosingRamContract variable, resulting in users not being able to update the characteristics of their NFTs via ChoosingRam::increaseValuesOfParticipants. This would break one of the core invariants of the protocol.

Tools Used

manual review, VSC

Recommendations

Consider deleting RamNFT::setChoosingRamContract function and making the following changes to RamNFT.sol:

+ import {ChoosingRam} from "./ChoosingRam.sol";
​
contract RamNFT is ERC721URIStorage {
....
....
- address public choosingRamContract;
+ ChoosingRam public choosingRamContract;
...
...
modifier onlyChoosingRamContract() {
- if (msg.sender != choosingRamContract) {
+ if (msg.sender != address(choosingRamContract)) {
revert RamNFT__NotChoosingRamContract();
}
_;
}
- constructor() ERC721("RamNFT", "RAM") {
+ constructor(address _choosingRamContract) ERC721("RamNFT", "RAM") {
tokenCounter = 0;
organiser = msg.sender;
+ choosingRamContract = ChoosingRam(_choosingRamContract);
}
​
- function setChoosingRamContract(address _choosingRamContract) public onlyOrganiser {
- choosingRamContract = _choosingRamContract;
- }
Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
v1vah0us3 Submitter
about 1 year ago
bube Lead Judge
about 1 year ago
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.