withdrawFee() can be called indefinitely

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L102-L105 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L76-L79

Vulnerability details

The function withdrawFee() does not account whether the fees have already been collected or not, therefore it can be called multiple times or even indefinitely, until the contract balance reaches zero. All funds will be transferred to the protocolFeeRecipient, but the participating users will not be able to get their reward. On a side note: the function use the onlyAdminWithdrawAfterEnd(), which only checks whether the endTime has been reached and there is no check on which user do the call, not sure if this is done by design. Anyway this mean that any user can call withdrawFee().

Impact

Contract can be drained (although the funds will go to protocolFeeRecipient), possibly denying the rewards or breaking the contract accounting.

Proof of Concept

In Erc20Quest.spec, copy and paste line 365 multiple times, for example:

await ethers.provider.send('evm_increaseTime', [100001])
await deployedQuestContract.withdrawFee()
await deployedQuestContract.withdrawFee() 
await deployedQuestContract.withdrawFee() 

You can see the call with go through and the contract balance will be less than expected

Tools Used

Manual check

Recommended Mitigation Steps

Make withdrawFee() callable only once

[c4-judge]

kirk-baird marked the issue as duplicate of #23

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird changed the severity to 3 (High Risk)