Newly added rewards will get frozen when a `unlock()` transaction get in between before `updateDistribution()`

[code423n4]

Handle

WatchPug

Vulnerability details

The current implementation requires the rewarder (usually the platform) to transfer the rewards (XDEFI tokens) to the contract and calls updateDistribution() to increase _pointsPerUnit, which will distribute the newly added rewards to all existing users according to the uints.

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L332

    function _updateXDEFIBalance() internal returns (int256 newFundsTokenBalance_) {
        uint256 previousDistributableXDEFI = distributableXDEFI;
        uint256 currentDistributableXDEFI = distributableXDEFI = IERC20(XDEFI).balanceOf(address(this)) - totalDepositedXDEFI;

        return _toInt256Safe(currentDistributableXDEFI) - _toInt256Safe(previousDistributableXDEFI);
    }

However, every unlock() transaction will call the internal function _updateXDEFIBalance(), which will then update the distributableXDEFI.

Therefore, when a unlock() transaction happened in between the transfer and the updateDistribution() transaction, the newly added rewards will not be recognized.

Actually, there is no way to get back these reward tokens. These tokens are frozen in the contract.

This can happen quite likely if the network is congested.

PoC

1. The rewarder transferred 1M XDEFI to the XDEFIDistribution contract;
2. A unlock() transaction by a regular user;
3. The rewarder calls updateDistribution().

Expected Results: 1M XDEFI get distributed to the users by increasing the _pointsPerUnit.

Actual Results: _pointsPerUnit remain unchanged. The 1M XDEFI sent by the rewarder in step 1 is now frozen in the contract.

Recommendation

Consider changing updateDistribution() to addRewards(uint amount), and use transferFrom() to pull tokens from the caller.

[deluca-mike]

Technically valid, but this was expected, and we disagree with severity, as explained in the duplicate issue #30.