26Q2 - Security Upgrades

[pankajjagtapp]

Note

High Risk
High risk because it changes authorization, pausing, and withdrawal/escrow accounting across core contracts (LiquidityPool, PriorityWithdrawalQueue, RedemptionManager, tokens), which can impact fund flows and upgrade safety if misconfigured.

Overview
Security hardening across core contracts. Replaces multiple ad-hoc admins/pausers mappings with RoleRegistry-based roles and updates scripts/tests accordingly, including new role checks for pausing/unpausing.

Adds new enforcement controls. Introduces pervasive IBlacklister checks (deposits, bids, redemptions, token transfers, membership actions) and adds timed pause support via PausableUntil (new PAUSE_UNTIL_ROLE/UNPAUSE_UNTIL_ROLE) wired into LiquidityPool, EtherFiRateLimiter, EtherFiNodesManager, PriorityWithdrawalQueue, and distributors.

Refactors withdrawal/escrow mechanics. LiquidityPool now supports an escrow migration (initializeOnUpgradeV2) that moves previously “locked” ETH out of LP into WithdrawRequestNFT/PriorityWithdrawalQueue, updates LP withdraw to treat NFT/queue withdrawals as paid from segregated balances, and adds new LP<->queue flows (transferLockedEthForPriority, returnLockedEth) with PriorityWithdrawalQueue updated to escrow ETH on fulfill/claim/cancel and enforce a new MAX_AMOUNT.

Additional guardrails. Adds a namespaced reentrancy guard (ReentrancyGuardNamespaced) to avoid proxy storage layout issues, enforces a minimum share value check in LiquidityPool, adds stETH market-value validation using a Chainlink feed + deviation/staleness bounds in Liquifier, and tightens EtherFiAdmin report validation plus a stale-oracle withdrawal finalization path.

Minor: CI now passes OP_RPC_URL, and foundry.lock is ignored.

^{Reviewed by Cursor Bugbot for commit 4d3febc. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Liquifier price deviation check divides by zero when market value is zero

Low Severity

In quoteByMarketValue, if the Curve pool's get_dy returns zero for a non-zero _amount (e.g., during extreme pool imbalance), _marketValue would be zero. The subsequent deviation * BASIS_POINT_SCALE / _marketValue division would then revert with an unhelpful panic instead of the expected InvalidStEthPrice error, making debugging harder and masking the root cause.

 

^{Reviewed by Cursor Bugbot for commit 1e11e8e. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 6 total unresolved issues (including 4 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit d442527. Configure here.}

[cursor]

Uninitialized rate limits block oracle report execution

High Severity

maxFinalizedWithdrawalAmountPerDay and maxNumValidatorsToApprovePerDay are new storage variables that default to 0 and have no initialization in the constructor, initialize, or any initializeOnUpgrade function. The _validateReport function checks numValidatorsToApprovePerDay > maxNumValidatorsToApprovePerDay and finalizedWithdrawalAmountPerDay > maxFinalizedWithdrawalAmountPerDay, so any oracle report containing non-zero validator approvals or finalized withdrawals will be rejected until an admin manually sets these values.

Additional Locations (1)

• src/EtherFiAdmin.sol#L318-L324

 

^{Reviewed by Cursor Bugbot for commit d442527. Configure here.}

[cursor]

Validator approval rate limit can be set to zero

Medium Severity

updateMaxNumValidatorsToApprovePerDay only checks > MAX_NUM_VALIDATORS_TO_APPROVE_PER_DAY but lacks a == 0 lower bound check, unlike updateMaxFinalizedWithdrawalAmountPerDay which explicitly rejects zero. Setting this to 0 permanently blocks all validator approvals through oracle reports, since any non-zero count will fail _validateReport.

 

^{Reviewed by Cursor Bugbot for commit d442527. Configure here.}