blackhole - The pool.lastRewardBlock should be updated in the setStartBlock function

[sherlock-admin3]

blackhole

medium

The pool.lastRewardBlock should be updated in the setStartBlock function

Summary

The admin can update the startBlock using the setStartBlock function.
However, during this update, pool.lastRewardBlock is not modified.
This can lead to incorrect pendingPoints calculations if the startBlock is set to an earlier block.
As a result, the pendingPoints will not be correctly distributed starting from the new startBlock.

Vulnerability Detail

File: contracts/farm/SophonFarming.sol#L279
    function setStartBlock(uint256 _startBlock) public onlyOwner {
        if (_startBlock == 0 || (endBlock != 0 && _startBlock >= endBlock)) {
            revert InvalidStartBlock();
        }
        if (getBlockNumber() > startBlock) {
            revert FarmingIsStarted();
        }
        startBlock = _startBlock; // @audit pool.lastRewardBlock should be updated...
    }
File: contracts/farm/SophonFarming.sol#L163
    function add(uint256 _allocPoint, address _lpToken, string memory _description, bool _withUpdate) public onlyOwner returns (uint256) {
        ...
        uint256 lastRewardBlock =
            getBlockNumber() > startBlock ? getBlockNumber() : startBlock;
        ...
    }
    

Impact

The pendingPoints will not be correctly distributed starting from the new startBlock.

Code Snippet

contracts/farm/SophonFarming.sol#L279

Tool used

Manual Review

Recommendation

It is recommended to update the poolInfo.lastRewardBlock in the setStartBlock function.

File: contracts/farm/SophonFarming.sol#L272
    function setStartBlock(uint256 _startBlock) public onlyOwner {
        if (_startBlock == 0 || (endBlock != 0 && _startBlock >= endBlock)) {
            revert InvalidStartBlock();
        }
        if (getBlockNumber() > startBlock) {
            revert FarmingIsStarted();
        }
        startBlock = _startBlock;

+        uint256 length = poolInfo.length;
+        for (uint256 pid = 0; pid < length; ++pid;) {
+            if (getBlockNumber() < poolInfo[pid].lastRewardBlock)
+                poolInfo[pid].lastRewardBlock = startBlock;
+        }
    }

Duplicate of #108

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

valid because lastRewardBlock for each pool should indeed sync with the latest startBlock

[mystery0x]

This report should be valid as it described scenario 2 of #108.