Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L356
https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L337

Vulnerability details

As arbitrary ERC20 tokens can be passed, the amount here should be calculated every time to take into consideration a possible fee-on-transfer or deflation.
Also, it's a good practice for the future of the solution.

Affected code:

• File: Booster.sol

345:     function deposit(
346:         uint256 _pid,
347:         uint256 _amount,
348:         bool _stake
349:     ) public returns (bool) {
...
356:         IERC20(lptoken).safeTransferFrom(msg.sender, staker, _amount); //@audit medium: not compatible with Fee On Transfer Tokens
...
372:             ITokenMinter(token).mint(address(this), _amount);
...
374:             IERC20(token).safeApprove(rewardContract, _amount);
375:             IRewards(rewardContract).stakeFor(msg.sender, _amount);
...
378:             ITokenMinter(token).mint(msg.sender, _amount);
...
381:         emit Deposited(msg.sender, _pid, _amount);
...

• File: VE3DRewardPool.sol

336:     function donate(address _rewardToken, uint256 _amount) external {
337:         IERC20(_rewardToken).safeTransferFrom(msg.sender, address(this), _amount); //@audit medium: not compatible with Fee On Transfer Tokens
338:         rewardTokenInfo[_rewardToken].queuedRewards += _amount;
339:     }

Recommended Mitigation Steps

Use the balance before and after the transfer to calculate the received amount instead of assuming that it would be equal to the amount passed as a parameter.

[solvetony]

Confirmed in donate function, we are have to add a check for the reward token list
However, I don't think we do anything with lp token as we already use the exact lp token address

[GalloDaSballo]

While I think the scenario is highly unlikely, the warden has shown how, in the case of a feeOnTransfer token being added as reward, the claiming of reward could be potentially broken.

Remediation can be as simple as checking for the actual transferred amount in the donate function, or simply not allowing the tokens.

However, because the warden has shown how the system could be bricked due to a configuration issue, I believe Medium Severity to be appropriate