Immunefi Audit Competitions
Plume | Attackathon

51651 sc insight redundant array access in removestakerfromvalidator

Submitted on Aug 4th 2025 at 16:13:23 UTC by @AasifUsmani for Attackathon | Plume Network

Description

Brief/Intro

In the removeStakerFromValidator function of the PlumeValidatorLogic library, an unnecessary array access occurs during the removal of a staker from a validator's list. Specifically, the declaration and access of lastStaker happen unconditionally, even when no swap is needed (i.e., when removing the last element). This leads to a redundant storage read (SLOAD) that can be avoided by moving the access inside the conditional that performs the swap.

Vulnerability Details

The issue is in Part 1 of the function, which uses a swap-and-pop pattern to remove a staker from $.validatorStakers[validatorId]:

if (indexToRemove < listLength && stakersList[indexToRemove] == staker) {
    address lastStaker = stakersList[listLength - 1];  // Redundant if no swap is needed
    if (indexToRemove != listLength - 1) {
        stakersList[indexToRemove] = lastStaker;
        $.userIndexInValidatorStakers[lastStaker][validatorId] = indexToRemove;
    }
    stakersList.pop();
}
  • The line address lastStaker = stakersList[listLength - 1]; is executed regardless of whether a swap will occur.
  • If indexToRemove == listLength - 1 (removing the last element), no swap is needed — only pop() is required. In that case, lastStaker is never used, making the read unnecessary.
  • This results in a redundant SLOAD, costing gas without changing behavior.
  • Functionally the code is correct; this is a low-risk optimization opportunity.

Severity: Low / Informational — No security risk or incorrect state change. Optimization and clarity improvement only.

Impact Details

  • Gas Efficiency: Each redundant SLOAD adds gas (roughly 100–200 gas per call). In high-frequency removal scenarios (mass unstaking, bulk operations), this can accumulate.
  • Code Quality: Removing the redundant read improves clarity and aligns with standard swap-and-pop patterns in Solidity, reducing potential confusion for maintainers and auditors.

Move the lastStaker declaration inside the conditional that performs the swap so it is only read when needed:

if (indexToRemove < listLength && stakersList[indexToRemove] == staker) {
    if (indexToRemove != listLength - 1) {
        address lastStaker = stakersList[listLength - 1];
        stakersList[indexToRemove] = lastStaker;
        $.userIndexInValidatorStakers[lastStaker][validatorId] = indexToRemove;
    }
    stakersList.pop();
}

This avoids the unnecessary storage read when removing the last element.

References

https://github.com/immunefi-team/attackathon-plume-network/blob/580cc6d61b08a728bd98f11b9a2140b84f41c802/plume/src/lib/PlumeValidatorLogic.sol#L94

Proof of Concept

Consider a validator's stakersList with 3 elements: [0xA, 0xB, 0xC] (length = 3).

Remove last element (0xC)

  • indexToRemove = 2
  • listLength = 3

Current code:

  • Reads lastStaker = stakersList[2] (0xC) unconditionally.
  • Checks if (2 != 2) → false, so no swap.
  • Pops the last element → resulting list: [0xA, 0xB]

Redundancy: lastStaker was read but never used.

Optimized flow

With the recommended change:

  • The code checks if (indexToRemove != listLength - 1) first.
  • Since it's false, it skips reading stakersList[listLength - 1].
  • Calls pop() → resulting list: [0xA, 0xB]

Benefit: One less SLOAD (gas saved) when removing the last element.