Summary

Inconsistent in the modifiers used in the Dussehra.sol::withdraw() preventing the function to be called

Vulnerability Details

The code has 3 modifiers about ram, one in choosingRam.sol is choosingRam.sol::RamIsNotSelected()

This modifier ensures that the function choosingRam.sol::selectRamIfNotSelected() can only be called when isRamSelected equals to false.

Next, In the Dussehra.sol we got Dussehra.sol::RamIsSelected() and Dussehra.sol::OnlyRam()

Both of these modifiers in Dussehra.sol are used for the Dussehra.sol::withdraw(), but both of them can cause trouble by themselves because of a condition, let's go back to choosingRam.sol::increaseValuesOfParticipants(), in here once a person successfully get the ram attribute, a variable with the name of selectedRam is updated to store the address of the new ram, making the Dussehra.sol::OnlyRam() pass when the address of ram calling the withdraw, but the withdraw won't continue, here is why. We need to take a look at the Dussehra.sol::withdraw() function,

In that function, notice that OnlyRam is not the modifier used on the function (we are going to ignore RavanKilled), we got another modifier called RamIsSelected, even though the selectedRam variable has some value (let's say msg.sender here so we pass the modifier), the ram cannot call the withdraw because the isRamSelected bool is still false, the only way to update this bool is an organizer calling the function choosingRam::selectRamIfNotSelected(). Only then the randomly chosen ram can call the withdraw function.

Impact

Even though a ram is already selected via the choosingRam.sol::increaseValuesOfParticipants() after successfully updating one characteristics and became ram, if the event continues until the end without the organization involvement of calling the choosingRam::selectRamIfNotSelected(), the money will be forever stuck in the contract since nobody can withdraw it due to the modifier.

Tools Used

Manual Analysis

Recommendations

It is better to set the bool isRamSelected to become true after the update of user, so that the modifier will also become true and not stopping the withdraw process. Adding this to the code choosingRam.sol::increaseValuesOfParticipants() is recommended