Rewards can be retrieved in a almost flash loan manner

Summary

As there are no delays neither in new lock creation nor in rewards claiming a big deposit with the shortest possible duration from an attacker can back-run the reward token transfer and instantly claim the most part of the rewards pot, without actually locking the funds for any viable duration.

Vulnerability Detail

An attacker can roll the lock with the minimal 10 minutes duration and back-run the distributeRewards() call with supplying vast capital amount with increaseLock() for the remainder of his short term lock duration, inflating the shares and immediately retrieving the rewards.

The attacker can optimize here, either calling right after distributeRewards() if there is a competition from other MEW attackers, or calling right before his lock expires if there isn't and capital lock time can be cut down to 12 seconds.

I.e. the lower curve point that will apply still does provide shares exposure, while the remainder of 10 minutes can be low enough for the cost of capital for that period to be not substantial compared to the market value of reward tokens allocated this way.

Impact

The attacker inflates the shares, obtaining the majority of rewards without any useful capital provision.

Net impact is the loss of the rewards for all the other stakers as the shares will be severely inflated for the given total reward amount.

Code Snippet

distributeRewards() provides the funds:

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/BasePool.sol#L95-L98

    function distributeRewards(uint256 _amount) external override {
        rewardToken.safeTransferFrom(_msgSender(), address(this), _amount);
        _distributeRewards(_amount);
    }

After that rewards can be withdrawn immediately:

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/BasePool.sol#L100-L112

    function claimRewards(address _receiver) external {
        uint256 rewardAmount = _prepareCollect(_msgSender());
        uint256 escrowedRewardAmount = rewardAmount * escrowPortion / 1e18;
        uint256 nonEscrowedRewardAmount = rewardAmount - escrowedRewardAmount;

        if(escrowedRewardAmount != 0 && address(escrowPool) != address(0)) {
            escrowPool.deposit(escrowedRewardAmount, escrowDuration, _receiver);
        }

        // ignore dust
        if(nonEscrowedRewardAmount > 1) {
            rewardToken.safeTransfer(_receiver, nonEscrowedRewardAmount);
        }

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/AbstractRewards.sol#L106-L107

  function _prepareCollect(address _account) internal returns (uint256) {
    uint256 _withdrawableDividend = withdrawableRewardsOf(_account);

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/AbstractRewards.sol#L53-L55

  function withdrawableRewardsOf(address _account) public view override returns (uint256) {
    return cumulativeRewardsOf(_account) - withdrawnRewards[_account];
  }

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/AbstractRewards.sol#L73-L75

  function cumulativeRewardsOf(address _account) public view override returns (uint256) {
    return ((pointsPerShare * getSharesOf(_account)).toInt256() + pointsCorrection[_account]).toUint256() / POINTS_MULTIPLIER;
  }

I.e. there are no delays anywhere, as shares are issued immediately and rewards can be claimed immediately as well, while 10 minutes window is small enough so a big amount of funds can be allocated for that period only by the attacker.

Tool used

Manual Review

Recommendation

Consider forcing min lock time in increaseLock():

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/TimeLockPool.sol#L210-L218

        depositToken.safeTransferFrom(_msgSender(), address(this), _increaseAmount);

        // Multiplier should be acording the remaining time to the deposit to end
        uint256 remainingDuration = uint256(userDeposit.end - block.timestamp);
+       require(remainingDuration > MIN_LOCK_DURATION);

        uint256 mintAmount = _increaseAmount * getMultiplier(remainingDuration) / 1e18;

        depositsOf[_receiver][_depositId].amount += _increaseAmount;
        depositsOf[_receiver][_depositId].shareAmount += mintAmount;

Consider adding the delays for the rewards claiming.

Also, MIN_LOCK_DURATION can be increased as 10 minutes isn't meaningful for capital allocation purpose and the only usage of such short locks is manipulation.

Duplicate of #106

sherlock-audit / 2022-10-merit-circle-judging

hyh - Rewards can be retrieved in a almost flash loan manner #121