49817 sc medium inactive validators are prevented to claim to eligible commission rewards

  • Report ID: #49817

  • Report Type: Smart Contract

  • Report severity: Medium

  • Target: https://github.com/immunefi-team/attackathon-plume-network/blob/main/plume/src/facets/ValidatorFacet.sol

  • Impacts:

    • Temporary freezing of funds for at least 24 hours

Description

Summary

Validators who have initiated the commission claim process via requestCommissionClaim and waited the mandatory one-week period cannot execute finalizeCommissionClaim to withdraw their earned rewards when an administrator changes their status to inactive.

Vulnerability Details

While a validator has an active status and participates in the network, they earn commission rewards. The intended flow is:

1

Step: requestCommissionClaim

Validator calls requestCommissionClaim to initiate the commission claim process.

Reference: https://github.com/plumenetwork/contracts/blob/fe67a98fa4344520c5ff2ac9293f5d9601963983/plume/src/facets/ValidatorFacet.sol#L500-L539

function requestCommissionClaim(
    uint16 validatorId,
    address token
)
    external
    onlyValidatorAdmin(validatorId)
    nonReentrant
    _validateValidatorExists(validatorId)
    _validateIsToken(token)
{
   ...
    $.pendingCommissionClaims[validatorId][token] = PlumeStakingStorage.PendingCommissionClaim({
        amount: amount,
        requestTimestamp: nowTs,
        token: token,
        recipient: recipient
    });
    $.validatorAccruedCommission[validatorId][token] = 0;
    ...
}
2

Step: waiting period

Wait for the mandatory one-week period to pass:

claim.requestTimestamp + PlumeStakingStorage.COMMISSION_CLAIM_TIMELOCK

3

Step: finalizeCommissionClaim

Validator calls finalizeCommissionClaim to withdraw their earned rewards.

Reference: https://github.com/plumenetwork/contracts/blob/fe67a98fa4344520c5ff2ac9293f5d9601963983/plume/src/facets/ValidatorFacet.sol#L573-L575

function finalizeCommissionClaim(
    uint16 validatorId,
    address token
) external onlyValidatorAdmin(validatorId) nonReentrant returns (uint256) {
    ...
    uint256 readyTimestamp = claim.requestTimestamp + PlumeStakingStorage.COMMISSION_CLAIM_TIMELOCK;

    // First, check if the timelock has passed from the perspective of the current block.
    if (block.timestamp < readyTimestamp) {
        revert ClaimNotReady(validatorId, token, readyTimestamp);
    }

    // --- REVISED SLASHING CHECK ---
    // If the validator is slashed, the claim is only considered valid if its timelock was
    // fully completed BEFORE the slash occurred. This invalidates any pending claims.
    if (validator.slashed && readyTimestamp >= validator.slashedAtTimestamp) {
        revert ValidatorInactive(validatorId);
    }

    // For a non-slashed validator, simply require it to be active to finalize a claim.
    if (!validator.slashed && !validator.active) {
        revert ValidatorInactive(validatorId);
    }
    ...
}

The problem: Even if the validator was eligible to finalize the claim after the waiting period, if an admin sets the validator to inactive after the claim became eligible, finalizeCommissionClaim will revert with ValidatorInactive. This prevents the validator from withdrawing funds that were accrued while active and whose claim became eligible before the status change.

This is inconsistent with other behaviors in the codebase (e.g., users can unstake and withdraw from inactive validators and can collect rewards/funds from slashed validators if those funds were in a parked state eligible to be claimed before the slash).

Impact

Any validator that made a commission claim and became eligible to collect it before being set to inactive can be prevented from collecting those rewards. This results in temporary freezing of funds that can only be resolved if the admin re-activates the validator.

Recommendation

Modify finalizeCommissionClaim so it allows finalization when the claim timelock completed before the validator became inactive. Suggested change:

function finalizeCommissionClaim(
    uint16 validatorId,
    address token
) external onlyValidatorAdmin(validatorId) nonReentrant returns (uint256) {
    ...

    // For a non-slashed validator, simply require it to be active to finalize a claim.
-    if (!validator.slashed && !validator.active) {
-        revert ValidatorInactive(validatorId);
-    }

+    bool validatorCanClaimRewards = (claim.requestTimestamp + PlumeStakingStorage.COMMISSION_CLAIM_TIMELOCK) < validator.slashedAtTimestamp;
+    if (!validator.slashed && !validatorCanClaimRewards) {
+        revert ValidatorInactive(validatorId);
+    }
    ...
}

This ensures a non-slashed validator may finalize a claim if the claim's timelock completed before the validator was marked inactive (or slashed).

Proof of Concept

Context

PoC demonstrates:

1

Active validator requests commission claim.

2

Active validator waits the unlock period (7 days) and becomes eligible to claim.

3

Admin sets the validator to inactive.

4

Validator tries to claim commission but finalizeCommissionClaim reverts with ValidatorInactive.

PoC (test)

Add the following test in PlumeStakeDiamond.t.sol:

Show test code (click to expand)
function testValidatorCannotClaimEligibleCommission_whenSetToInactive() public {
        // Set a very specific reward rate for predictable results
        uint256 rewardRate = 1e18; // 1 PUSD per second
        vm.startPrank(admin);
        address[] memory tokens = new address[](1);
        tokens[0] = address(pUSD);
        uint256[] memory rates = new uint256[](1);
        rates[0] = rewardRate;
        RewardsFacet(address(diamondProxy)).setRewardRates(tokens, rates);

        // Make sure treasury is properly set
        RewardsFacet(address(diamondProxy)).setTreasury(address(treasury));

        // Ensure treasury has enough PUSD by transferring tokens
        uint256 treasuryAmount = 100 ether;
        pUSD.transfer(address(treasury), treasuryAmount);
        vm.stopPrank();

        // Set a 10% commission rate for the validator
        vm.startPrank(validatorAdmin);
        uint256 newCommission = 10e16;
        ValidatorFacet(address(diamondProxy)).setValidatorCommission(
            DEFAULT_VALIDATOR_ID,
            newCommission
        );
        vm.stopPrank();

        // Create validator with 10% commission
        uint256 initialStake = 10 ether;
        vm.startPrank(user1);
        StakingFacet(address(diamondProxy)).stake{value: initialStake}(
            DEFAULT_VALIDATOR_ID
        );
        vm.stopPrank();

        // Move time forward to accrue rewards
        vm.roll(block.number + 10);
        vm.warp(block.timestamp + 10);

        // Trigger reward updates by having a user interact with the system
        // This will internally call updateRewardsForValidator
        vm.startPrank(user2);
        StakingFacet(address(diamondProxy)).stake{value: 1 ether}(
            DEFAULT_VALIDATOR_ID
        );
        vm.stopPrank();

        // Move time forward again
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + 1);

        // Interact again to update rewards once more
        vm.startPrank(user1);

        // Unstake a minimal amount to trigger reward update
        StakingFacet(address(diamondProxy)).unstake(DEFAULT_VALIDATOR_ID, 1); // Unstake 1 wei
        vm.stopPrank();

        // Check that some commission has accrued (positive amount)
        uint256 commission = ValidatorFacet(address(diamondProxy))
            .getAccruedCommission(DEFAULT_VALIDATOR_ID, address(pUSD));
        assertGt(commission, 0, "Commission should be greater than 0");

        // @audit - validator request commission claim
        vm.startPrank(validatorAdmin);
        uint256 balanceBefore = pUSD.balanceOf(validatorAdmin);

        ValidatorFacet(address(diamondProxy)).requestCommissionClaim(
            DEFAULT_VALIDATOR_ID,
            address(pUSD)
        );
        vm.stopPrank();

        // unlock period passes(7 days), now this validator is eligible to claim commission
        uint256 blockTimestamp = block.timestamp;
        vm.warp(blockTimestamp + 7 days + 1 seconds);

        // but admin makes validator inactive first
        vm.startPrank(admin);
           ValidatorFacet(address(diamondProxy)).setValidatorStatus(
            DEFAULT_VALIDATOR_ID,
            false
        );
        vm.stopPrank();

        // @audit - validator try to finalize commission claim but it reverts with ValidatorInactive error
        vm.startPrank(validatorAdmin);
            vm.expectRevert(
                abi.encodeWithSelector(ValidatorInactive.selector, DEFAULT_VALIDATOR_ID)
            );
            ValidatorFacet(address(diamondProxy)).finalizeCommissionClaim(
                DEFAULT_VALIDATOR_ID,
                address(pUSD)
            );
        vm.stopPrank();
    }

Run: forge test --mt testValidatorCannotClaimEligibleCommission_whenSetToInactive -vv --via-ir

Output:

[PASS] testValidatorCannotClaimEligibleCommission_whenSetToInactive() (gas: 1598430)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 12.36ms (1.18ms CPU time)
Ran 1 test suite in 182.91ms (12.36ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

If you want, I can:

  • Propose a minimal patched implementation for finalizeCommissionClaim that accounts for validator inactive transitions (keeping style and existing checks).

  • Create a unit test that validates both scenarios (claim eligible before inactive vs not eligible).

Was this helpful?