Description

When queueValidatorRemoval runs on a vault with principalDeposits equal to zero but containing rewards, these rewards remain unclaimed and are lost in accounting because they are not added to the second entry of ValidatorRemoval:

function queueValidatorRemoval(uint256 _validatorId) external onlyOwner {
    // [...]
    if (principalDeposits != 0) {
        uint256 preBalance = token.balanceOf(address(this));
        vault.unbond(principalDeposits);
        uint256 rewardsClaimed = token.balanceOf(address(this)) - preBalance;
        if (rewardsClaimed != 0) totalQueued += rewardsClaimed;
    }
    // [... remaining logic does not transfer vault token balance]
}

The finalization of the validator removal also does not withdraw the tokens held by the vault. So those tokens are lost during this process.

Impact

The impact is minimal. We identified two ways a token balance can unexpectedly exist that is owned by the PolygonVault contract.

The first case is if a user directly donates tokens to the contract. Although it would maximize yield to transfer such tokens, it is reasonable to not bother collecting them.

The second case is if a user donates a small quantity of shares to the contract. The implementation of the upstream ValidatorShare contract has a transfer hook that claims the rewards on behalf of both the sender and the recipient:

function _transfer(address to, uint256 value, bool pol) internal {
    address from = msg.sender;
    // get rewards for recipient
    _withdrawAndTransferReward(to, pol);
    // convert rewards to shares
    _withdrawAndTransferReward(from, pol);
    // move shares to recipient
    super._transfer(from, to, value);
    _getOrCacheEventsHub().logSharesTransfer(validatorId, from, to, value);
}

This means if an attacker sends an insignificant amount of shares to the PolygonVault, it will claim the rewards and send them to the vault. However, all code paths that lead to a principalDeposits of zero include claiming the existing rewards and transferring the standing balance back to the PolygonStrategy. So the only rewards that may accrue are from donated shares during the validator-removal process, which it is also reasonable to not collect.

Recommendations

We recommend adding a check in the finalizeValidatorRemoval function that withdraws and queues any standing token balance in the PolygonVault contract, even if getQueuedWithdrawals is zero.

Remediation

This issue has been acknowledged by Stake.link, and a fix was implemented in commit a675197e↗.