Lines of code

Vulnerability details

Impact

SocializingPool.verifyProof currently incorporates a zero index check which blocks the entry of an index value of 0. While this check is designed to prevent the use of invalid index values, it inadvertently prohibits the valid index value of 0. This may lead to undesired outcomes and it’s vital to rectify this problem to guarantee the contract operates correctly.

Proof of Concept

verifyProof() tests whether _index equals 0 or exceeds lastReportedRewardsData.index. However, the _index == 0 validation is superfluous and needs to be removed. This exclusion of the index value 0 may cause difficulties when dealing with the contract in missing out the zero indexed cycle.

SocializingPool.sol#L163-L176

    function verifyProof(
        uint256 _index,
        address _operator,
        uint256 _amountSD,
        uint256 _amountETH,
        bytes32[] calldata _merkleProof
    ) public view returns (bool) {
        if (_index == 0 || _index > lastReportedRewardsData.index) {
            revert InvalidCycleIndex();
        }
        bytes32 merkleRoot = rewardsDataMap[_index].merkleRoot;
        bytes32 node = keccak256(abi.encodePacked(_operator, _amountSD, _amountETH));
        return MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node);
    }

It’s also worth noting that the _index == 0 check is not in line with the fact that the current reward index can indeed be 0. This zero index will first be added 1:

SocializingPool.sol#L187-L189

    function getCurrentRewardsIndex() public view returns (uint256 index) {
        index = lastReportedRewardsData.index + 1;
    }

and then reflected herewith:

SocializingPool.sol#L191-L203

    function getRewardDetails()
        external
        view
        override
        returns (
            uint256 currentIndex,
            uint256 currentStartBlock,
            uint256 currentEndBlock
        )
    {
        currentIndex = getCurrentRewardsIndex();
        (currentStartBlock, currentEndBlock) = getRewardCycleDetails(currentIndex);
    }

and eventually show up as _index == 1 on line 212:

SocializingPool.sol#L205-L227

    /// @param _index reward cycle index for which details is required
    function getRewardCycleDetails(uint256 _index) public view returns (uint256 _startBlock, uint256 _endBlock) {
        if (_index == 0) {
            revert InvalidCycleIndex();
        }
        uint256 cycleDuration = staderConfig.getSocializingPoolCycleDuration();
        // for 1st cycle
212:        if (_index == 1) {
            return (initialBlock, initialBlock + cycleDuration);
        }

        // for past cycles
        _startBlock = rewardsDataMap[_index - 1].reportingBlockNumber + 1;
        _endBlock = rewardsDataMap[_index].reportingBlockNumber;

        // for current cycle
        if (rewardsDataMap[_index].reportingBlockNumber == 0) {
            if (rewardsDataMap[_index - 1].reportingBlockNumber == 0) {
                revert FutureCycleIndex();
            }
            _endBlock = rewardsDataMap[_index - 1].reportingBlockNumber + cycleDuration;
        }
    }

Recommended Mitigation Steps

It is suggested refactoring the impacted code by removing _index == 0 from the if block in verifyProof().

Assessed type

Error

---

The text was updated successfully, but these errors were encountered:

All reactions