Producer role not removed completely at the end of special event

Severity

Medium Risk

Relevant GitHub Links

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

Summary

When users join a special event, they are automatically assigned the producer role, which is recorded in the isProducer mapping and the producer array. However, when the stopEvent function is called to end the event, only the isProducer mapping is reset, while the producer array is not updated accordingly.

Vulnerability Details

The vulnerability lies in the incomplete removal of producer roles when the stopEvent function is executed. The function correctly resets the isProducer mapping, which tracks whether a user is a producer or not. However, it fails to remove the user from the producer array, which maintains a list of all producers.

This inconsistency can lead to several issues:

Stale Data: The producer array will contain outdated information, as it will still include users who are no longer producers after the event has ended.

Incorrect Producer Count: The length of the producer array will not accurately reflect the current number of active producers, as it will include users who have been removed from the isProducer mapping..

Potential Privilege Escalation: If the producer array is used for any privileged operations or access control, users who are no longer producers according to the isProducer mapping may still be able to perform actions reserved for producers.

 function stopEvent() external onlyOwner {
        require(block.timestamp >= eventEndTime, "Event is not ended");
        //@audit producer role  not totally removed, they are still in the array
        for (uint256 i = 0; i < participants.length; i++) {
            isProducer[participants[i]] = false;
        }
    }

Impact

The impact of this vulnerability depends on how the producer array is used within the smart contract and any associated systems. Potential consequences include:

1. Data Inconsistency: The presence of stale data in the producer array can lead to confusion and incorrect assumptions about the current state of producers.
2. Incorrect Calculations: If the producer array is used for any calculations or metrics, the results may be inaccurate due to the inclusion of users who are no longer producers.
3. Unauthorized Access: If the producer array is used for access control or privileged operations, users who are no longer producers according to the isProducer mapping may still be able to perform actions they should not have access to.

POC

The joinEvent calls the add _addProducer function

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);
    }

This function performs two actions, add user to the isProducer mapping and add user to the array
 function _addProducer(address _producer) internal {
        isProducer[_producer] = true;
        producers.push(_producer);
        console.log(producers[0]);
        // @audit no event for adding producer
    }

// The stopEvent only reset the IsProducer mapping
 function stopEvent() external onlyOwner {
        require(block.timestamp >= eventEndTime, "Event is not ended");
        //@audit producer role  not totally removed, they are still in the array
        for (uint256 i = 0; i < participants.length; i++) {
            isProducer[participants[i]] = false;
        }
        
    }

NB - The _addProducer contains a bug that is mentioned in another issue

Tools Used

Manual Review, Foundry

Recommendations

All users in the special event should be removed from the producers array in the stopEvent function.