Logic flaw in MartenitsaEvent::stopEvent function prevents users from participating in future events

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2024-04-Baba-Marta/blob/5eaab7b51774d1083b926bf5ef116732c5a35cfd/src/MartenitsaEvent.sol#L60

Summary

The MartenitsaEvent contract manages event logic within the protocol, primarily through functions like startEvent, joinEvent, and stopEvent.

The joinEvent function records participants and producers upon user entry to an event. However, the issue arises in the stopEvent function, where proper reversal of changes made in joinEvent is not handled correctly.

While joinEvent mandates that users are not already participants, this status is not reset for future events during stopEvent execution, rendering it impossible for a user to join subsequent events

Vulnerability Details

The function MartenitsaEvent::joinEvent updates a list of _participants and invokes the _addProducer function, temporarily adding the user as a producer

function joinEvent() external {
        require(block.timestamp < eventEndTime, "Event has ended");
@>      require(!_participants[msg.sender], "You have already joined the event");
        require(!isProducer[msg.sender], "Producers are not allowed to participate");
        require(_healthToken.balanceOf(msg.sender) >= healthTokenRequirement, "Insufficient HealthToken balance");
        
        _participants[msg.sender] = true;
        participants.push(msg.sender);
        emit ParticipantJoined(msg.sender);
        
        (bool success) = _healthToken.transferFrom(msg.sender, address(this), healthTokenRequirement);
        require(success, "The transfer is not successful");
        _addProducer(msg.sender);
    }

When it's time to conclude the event, the stopEvent function is invoked. However, it's noted that only the producer role is removed, while the participants remain unaffected.

		function stopEvent() external onlyOwner {
        require(block.timestamp >= eventEndTime, "Event is not ended");
        for (uint256 i = 0; i < participants.length; i++) {
@>		//@Audit: participants remain unchanged, only producer role
	          isProducer[participants[i]] = false;
        }
    }

A test is included to demonstrate a user joining an event, then attempting to join another event after the first one has concluded. This results in a revert of the joinEvent function due to the requirement check: require(!_participants[msg.sender], "You have already joined the event");

function testUserLockedEvent() public {
        //Give some health tokens so can join multiple events
        vm.startPrank(address(marketplace));        
        healthToken.distributeHealthToken(bob, 10 * 10**18);
        vm.stopPrank();


        //First event
        martenitsaEvent.startEvent(1 days);        
        
        vm.startPrank(bob);        
        healthToken.approve(address(martenitsaEvent), 10 ** 18);        
        martenitsaEvent.joinEvent();
        vm.warp(block.timestamp + 1 days + 1);
        vm.stopPrank();

        martenitsaEvent.stopEvent();

        //Time passes between events
        vm.warp(block.timestamp + 10 days);

        //Second event
        martenitsaEvent.startEvent(1 days);        


        vm.startPrank(bob);
        marketplace.collectReward();
        healthToken.approve(address(martenitsaEvent), 10 ** 18);        
        martenitsaEvent.joinEvent();
        vm.stopPrank();
    }

Output

Failing tests:
Encountered 1 failing test in test/MartenitsaMarketplace.t.sol:MartenitsaMarketplace
[FAIL. Reason: revert: You have already joined the event] testUserLockedEvent() (gas: 377973)

Impact

Users who have already participated in an event are unable to participate in future events.

Tools Used

Foundry and manual review

Recommendations

Removing participants from the list ensures they can join again in future events.

	 function stopEvent() external onlyOwner {
        require(block.timestamp >= eventEndTime, "Event is not ended");
        for (uint256 i = 0; i < participants.length; i++) {
            isProducer[participants[i]] = false;
++          _participants[participants[i]] = false;  
        }
    }