Lack of support for fee-on-transfer and rebasing ERC20 token

[code423n4]

Lines of code

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

Vulnerability details

Impact

Lack of support for fee-on-transfer and rebasing ERC20 token

Proof of Concept

.In the current implementation, the code assume the transfered amount matches the receiving amount

quest-protocol\contracts\Erc20Quest.sol:
   66      function _transferRewards(uint256 amount_) internal override {
   67:         IERC20(rewardToken).safeTransfer(msg.sender, amount_);
   68      }

   85          uint256 nonClaimableTokens = IERC20(rewardToken).balanceOf(address(this)) - protocolFee() - unclaimedTokens;
   86:         IERC20(rewardToken).safeTransfer(to_, nonClaimableTokens);
   87      }

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

However, according to

https://github.com/d-xo/weird-erc20#fee-on-transfer

Some tokens take a transfer fee (e.g. STA, PAXG), some do not currently charge a fee but may do so in the future (e.g. USDT, USDC).

If the ERC20 charge transfer fee, the user cannot claim the fully amount of reward they are entitled.

According to

https://github.com/d-xo/weird-erc20#balance-modifications-outside-of-transfers-rebasing--airdrops

Some tokens may make arbitrary balance modifications outside of transfers (e.g. Ampleforth style rebasing tokens, Compound style airdrops of governance tokens, mintable / burnable tokens).

Because the smart contract does not have method to handle the rebased ERC20 token or different airdropped ERC20 token, which can lock a part of token in the smart contract.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol make it explicit in the doc that the fee-on-transfer and rebasing ERC20 token are not supported.

[kirk-baird]

Closing as this seems to be exactly the same as #76

[c4-judge]

kirk-baird marked the issue as nullified