RabbitHole Quest Protocol contest - mahdikarimi's results

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L102-L104

Vulnerability details

Impact

protocolFeeRecipient can withraw fee multiple times without limit until there is no balance in contract

Proof of Concept

The only check applied to withdrawFee function is onlyAdminWithrawAfterEnd wich makes sure the endtime of quest has arrived due that users may claim their rewards after quest endtime the contract should have balance and due that this function can be called multiple times regardless of it's called before or not , even if admin didn't do that others can do at no cost because it's a public function .

Scenario1 : The quest has ended and some users didn't claim their rewards yet , admin calls withdrawFee function and sends protocolFee to feeReceipent and at the same time feeReceipent calls this function and even everyone can do this until there is no balance in contract and users that have not claimed to rewards will not be able to do this . .

Tools Used

Manual Review

Recommended Mitigation Steps

add a state boolean variable for example called feeClaimed and false value in time of construction which indicates fee has been claimed or not then declare a modifier that checks feeClaimed is false otherwise it should revert and add this modifier to withdrawFee function and at the end of the function execution set this variable to true , this way after one time call of this function It's not possible to call it again .