withdrawFee() can be called repeatedly by anyone to drain unclaimed rewards

[code423n4]

Lines of code

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

Vulnerability details

Impact

In Erc20Quest.sol, the withdrawFee() can be called multiple times by anyone to drain unclaimed rewards from the contract. This is due to missing tracking of previous protocol fee withdrawals and incorrect modifier.

Proof of Concept

After the end of the claim period, call the withdrawFee() multiple times to drain the contract of the unclaimed rewards. The function does not track or update the balance after transfering the protocol fee.

function withdrawFee() public onlyAdminWithdrawAfterEnd {
    IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee());
}

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

And there is no access control as the onlyAdminWithdrawAfterEnd modifier only ensure the withdrawFee() can be called after end of claim period.

modifier onlyAdminWithdrawAfterEnd() {
	if (block.timestamp < endTime) revert NoWithdrawDuringClaim();
	_;
}

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

Recommended Mitigation Steps

Track the amount of protocol fee withdrawn and add in access control if necessary.

[c4-judge]

kirk-baird marked the issue as duplicate of #23

[c4-judge]

kirk-baird marked the issue as satisfactory