Tokens may be stuck on the ERC20 quest contract

[code423n4]

Lines of code

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

Vulnerability details

Impact

If ProtocolFee were collected from ERC20 quest before calling withdrawRemainingTokens function, the amount of tokens to withdraw by withdrawRemainingTokens will be calculated wrong and tokens will be stuck on the contract.

Proof of Concept

Smart contract designed in a such way that creator of the quest may withdraw unused funds from the Quest after its ending.
The unused funds is calculates as Total balance on the contract less Unclaimed by participant less ProtocolFee.

 uint256 nonClaimableTokens = IERC20(rewardToken).balanceOf(address(this)) - protocolFee() - unclaimedTokens;

While amount of unclaimed tokens calculated at the moment of withdrawal, nature of ProtocolFee is a bit different.
ProtocolFee is static number calculated as percentage of the total amount of granted rewards. Contract does not check whether they were already withdrew from the contract or not.

So, if ProtocolFee were collected, their amount already subtracted from the balance of the ERC20 quest contract.
And thus while calculating nonClaimableTokens they would be subtracted second time and won't be withdrew from contract.
As these tokens are "free" and not claimable by participant, they would be stuck there forever.

Recommended Mitigation Steps

Before calculating nonClaimableTokens check whether protocolFee was already collected or not

[c4-judge]

kirk-baird marked the issue as duplicate of #61

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird changed the severity to 2 (Med Risk)