In ERC20Quest withdrawFee can be called multiple times by anyone

[code423n4]

Lines of code

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

Vulnerability details

Impact

The withdrawFee function in Erc20Quest is used to transfer the protocolFee to protocolFeeRecipient . This function has access control issues. The modifier onlyAdminWithdrawAfterEnd only checks if the function is called before endTime or not and reverts if it is.

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

This function is missing the onlyOwner modifier.

Due to this issue, it can be called multiple times by anyone to transfer funds to the protocolFeeRecipient, and the legitimate users would not be able to claim their tokens as there won’t be sufficient funds in the contract anymore.

Even if the protocolFeeRecipient transfer funds back to this contract, a malicious user with an objective to not let users claim their tokens, can front-run their claim call to transfer funds back to the protocolFeeRecipient and get their transactions reverted.

POC

function withdrawFee() public onlyAdminWithdrawAfterEnd {
        IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee()); //@audit anyone can call multiple times
    }

Recommendation Mitigation

Just add onlyOwner modifier.

[c4-judge]

kirk-baird marked the issue as duplicate of #23

[c4-judge]

kirk-baird marked the issue as satisfactory