Ctrlk
Active Boosts
For the complete documentation index, see llms.txt. This page is also available as Markdown.

88479 sc high rescue flags a still open withdrawal bucket permanently freezing the victim s later withdrawals for that period

Submitted on Aug 13th 2026 at 11:59:41 UTC by @FaithfulLayney for Audit Comp | Firelight

  • Report ID: #88479

  • Report Type: Smart Contract

  • Report severity: High

  • Target: https://github.com/immunefi-team/audit-comp-firelight/blob/v1_audit_ready/contracts/core/FirelightVault.sol

  • Impacts:

    • Permanent freezing of funds

Description

Brief/Intro

isWithdrawClaimed[period][account] is a terminal per-address flag: claimWithdraw checks it before computing any entitlement and reverts AlreadyClaimedPeriod for that (period, account) forever after. rescueWithdrawFromBlocklisted reuses the same flag to mean something different — "this address's credit was moved away" — and accepts any period, including currentPeriod() + 1, the bucket _requestWithdraw is still writing into. Once the flag is set on a live bucket the address is permanently poisoned for that period: after the compliance hold is lifted, any withdrawal credited to it for that period burns the user's shares, increments pendingWithdrawAssets, and can never be claimed. No reset path exists anywhere in the contract's own logic — only a new implementation could add one. In production this strands the caller's entire withdrawal — 5,000 fXRP, 100% of the request, in the PoC — with nobody gaining it.

Vulnerability Details

A withdrawal bucket p is OPEN while currentPeriod() == p-1 (the bucket index is fixed at :1008 and the credit written at :1017), SEALED while currentPeriod() == p, and CLAIMABLE once currentPeriod() > p (:791). The flag has exactly two writers and they disagree on its meaning: claimWithdraw sets it at :806 to mean "paid", and only ever on a CLAIMABLE bucket because :791 rejects period >= currentPeriod(); rescueWithdrawFromBlocklisted sets it at :870 to mean "moved away", on any bucket, with no bound relating periods[i] to currentPeriod():

// contracts/core/FirelightVault.sol:860-873
        uint256[] memory rescuedShares = new uint256[](len);
        for (uint256 i = 0; i < len; i++) {
            uint256 _withdrawOf = withdrawSharesOf[periods[i]][from];

            if (isWithdrawClaimed[periods[i]][from]) revert AlreadyClaimedPeriod(periods[i]);
            if (isWithdrawClaimed[periods[i]][to]) revert AlreadyClaimedPeriod(periods[i]);
            if (_withdrawOf == 0) revert NoWithdrawalAmount(periods[i]);

            withdrawSharesOf[periods[i]][to] += _withdrawOf;
            withdrawSharesOf[periods[i]][from] = 0;
            isWithdrawClaimed[periods[i]][from] = true;

            rescuedShares[i] = _withdrawOf;
        }

currentPeriod() + 1 is not an exotic argument: it is the only bucket a fresh pending withdrawal can be in, and the function's own NatSpec ("Rescues pending withdrawals") points at it. Meanwhile the most ordinary operation in the vault keeps crediting that same bucket for that same address, and burns the shares while doing it:

The sequence, as executed in the PoC: Alice requests a withdrawal into the open bucket p; Alice is blocklisted and RESCUER_ROLE rescues that pending withdrawal to Carol, setting the flag at :870; the compliance hold is lifted inside the same period; Alice makes an ordinary withdraw(), which credits p again and burns her shares; the bucket closes, Carol claims normally, and Alice's claimWithdraw(p) reverts AlreadyClaimedPeriod(p) forever.

Every recovery path is closed. Three are closed by assertion in the PoC: claimWithdraw reverts at :794; re-running the rescue reverts AlreadyClaimedPeriod at :864, because the flag it set is now its own precondition failure — the rescue cannot undo its own damage; and the ABI contains no non-view function matching /isWithdrawClaimed|resetWithdraw|clearWithdraw/. The fourth is closed by construction: rescueSharesFromBlocklisted moves ERC20 share balance, and the shares behind this request were already burned at :1025 — the PoC asserts that burn in step 4 via Alice's share balance rather than calling the function.

The intended protocol constraint that :870 bypasses is that isWithdrawClaimed is terminal by design: claimWithdraw may only ever set it on a bucket that can no longer receive credit, because :791 rejects period >= currentPeriod(). The rescue path is bound by no such rule, and neither role can see the damage — the call does not revert, no argument validation rejects the open bucket, and no documented procedure compares periods[i] against currentPeriod(). This is the exception the scope page's "Privileged actors" rule carves out; that rule reads, in full: "Any bug report that requires a privileged role holder to act within the permissions explicitly granted to that role is out of scope, unless the report demonstrates that the role can exceed its intended permissions or bypass an intended protocol constraint." Neither role needs to misbehave for the report to land on the second half — the constraint is bypassed by the code, not by the operator. And the transaction that actually loses the money is an unprivileged withdraw() by the victim herself.

The flag is not needed for safety, which makes the fix cheap: the sibling rescueSharesFromBlocklisted (:818-837) moves value and sets no terminal flag, so it never poisons the address it touches — a repeat call simply reverts InsufficientShares at :828 once the balance is zero. Zeroing withdrawSharesOf[periods[i]][from] at :869 already makes a second rescue revert NoWithdrawalAmount at :866 — the only protection :870 adds on the from side. Either bound the rescue to buckets that can no longer receive credit (if (periods[i] > currentPeriod()) revert InvalidPeriod(); at the top of the loop) or drop the :870 write entirely.

Impact Details

Permanent freezing of funds: 100% of the affected withdrawal request, with no path inside the contract that can ever release it and no beneficiary. The assets are not stolen — they are removed from the system, excluded from the user's claim and, because totalAssets() = super.totalAssets() - pendingWithdrawAssets (:427-429), from every LP's share value as well. The per-incident bound is the victim's entire remaining position: one ordinary withdraw() at the wrong moment strands whatever it requests. The poison is confined to bucket p and to that one address, so it bites only while p is still open — the remainder of one period, 604800 seconds in the sponsor's own fixture — and the victim's un-requested shares survive, still withdrawable to any other receiver or to herself once the period rolls. What no path inside the contract can recover, at any later time, is every asset credited to that address in that bucket in the meantime.

Measured in the PoC on the real FirelightVault proxy using the sponsor's own deployVault fixture (fXRP, 6 decimals):

Frozen forever

5,000.0 fXRP — 100.00% of the second request

Alice's remaining share balance

2,000,000,000 — unaffected, but her request is gone

pendingWithdrawAssets

5,000,000,000 — permanently overstated by exactly the frozen amount

Vault ERC20 balance

7,000,000,000

totalAssets()

2,000,000,000

Asserted invariant

vaultBalance - pendingWithdrawAssets == totalAssets() — the stranded tokens back nobody's shares

No external precondition is involved: no oracle, no price move, no other protocol, no MEV. The in-scope V2 implementation is not deployed yet, so no in-scope contract holds a live position at this moment.

References

Repository immunefi-team/audit-comp-firelight @ 42f9ea5e43d88b35197b901acc2a8c24b314fa62:

Proof of Concept

Step-by-step

Roles (fixture signers): users[0] = Alice (an ordinary LP), users[2] = carol (the safe address the rescue moves value to), RESCUER_ROLE, BLOCKLIST_ROLE. Bucket p = currentPeriod() + 1 is the OPEN bucket — the only one a fresh pending withdrawal can land in.

  1. Alice deposits. The open bucket is currentPeriod() + 1.

  2. Alice requests a withdrawal. _requestWithdraw fixes the bucket at currentPeriod() + 1 (FirelightVault.sol:1008), credits withdrawSharesOf[p][alice] (:1017), increases pendingWithdrawAssets (:1019) and burns her shares (:1025).

  3. Alice is blocklisted, and RESCUER_ROLE calls rescueWithdrawFromBlocklisted(alice, carol, [p]) — p is where her pending withdrawal is. Carol receives the credit and isWithdrawClaimed[p][alice] = true is written at :870. The call does not revert and nothing warns that p is still accepting credit.

  4. The compliance hold is lifted — removeFromBlocklist(alice), inside the same period.

  5. Alice makes an ordinary withdraw(). _requestWithdraw credits the same bucket p again, burns her shares again, and increases pendingWithdrawAssets again.

  6. The bucket closes. Carol claims normally. Alice's claimWithdraw(p) reverts AlreadyClaimedPeriod(p) — permanently, because :794 tests the flag before the entitlement is even computed at :796, and nothing anywhere clears it.

  7. No recovery exists — two paths closed by assertion, one by construction: re-running the rescue reverts AlreadyClaimedPeriod at :864 (asserted; the flag it set is now its own precondition failure); the ABI has no non-view function matching /isWithdrawClaimed|resetWithdraw|clearWithdraw/ (asserted); and rescueSharesFromBlocklisted cannot help because it moves ERC20 share balance, while the shares behind this request were burned at :1025 — the test asserts that burn in step 4 via Alice's share balance rather than calling the function.

  8. The assets leave the system, asserted as vaultBalance - pendingWithdrawAssets == totalAssets(): they are neither claimable by Alice nor counted in any LP's share value.

Net result: 5,000 fXRP — 100% of the second request — frozen forever, with nobody gaining it. No oracle, no price move, no other protocol, no MEV.

Runnable test

Reproduced from a clean checkout of the in-scope repository at the competition commit with this single file added and nothing else changed. All execution is local Hardhat; no transaction was sent to any live or public network.

Result: 8 passing. The test uses the repository's own test/setup/fixtures.js and the production FirelightVault behind the sponsor's own upgrades.deployProxy; the asset is the repository's vendored Flare FAsset token (contracts/fasset/fassetToken/implementation/FAsset.sol), and the only mock in the fixture is contracts/test/MockContract.sol, installed as that token's asset manager so it can mint. The sponsor's suite is unaffected: 448 passing on its own, 456 passing with this file added.

test/poc/020-rescue-marks-open-bucket.js:

Output (one run; the total runtime is machine-dependent, every other value is deterministic)

Previous89272 sc high stale rescue marker permanently freezes a later withdrawal
Next89281 sc high anyone can permanently destroy any staker s entire balance for 0 000001 fxrp

Was this helpful?