88595 sc high rescuing a not yet claimable withdrawal request permanently poisons the period account pair freezing any later request on that period
Submitted on Aug 14th 2026 at 02:02:31 UTC by @Kalila95146 for Audit Comp | Firelight
Report ID: #88595
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
rescueWithdrawFromBlocklisted marks the source account as already claimed for the rescued period. It accepts any period, including one that is not yet claimable — which is the normal case, since withdrawal requests always target currentPeriod() + 1.
_requestWithdraw never reads that flag. claimWithdraw does, and reverts on it.
So after a rescue, if the account files a new withdrawal request that lands on the same period, the contract accepts the request and burns the shares for a payout it can never honour. The flag is never cleared anywhere in the contract, so the assets are frozen permanently.
Vulnerability Details
FirelightVault uses delayed withdrawals: withdraw/redeem burn the caller's shares and create a pending request targeting currentPeriod() + 1, claimable later via claimWithdraw(period).
1 — The rescue sets a permanent flag, on any period
FirelightVault.sol:845-876:
There is no periods[i] < currentPeriod() check. Rescuing a request for a period that has not yet become claimable therefore sets isWithdrawClaimed[futurePeriod][from] = true.
2 — A new request never consults that flag
FirelightVault.sol:994-1028:
3 — But the claim does
FirelightVault.sol:788-811:
isWithdrawClaimed is never set back to false anywhere in the contract (verified: the only writes are :806 and :870, both to true). The pair (period, account) is poisoned permanently.
Why this is a contract defect, not an operational choice
The most likely objection is "the rescuer could simply have waited until the period became claimable". Four reasons that does not hold:
The function explicitly accepts any period.
rescueWithdrawFromBlocklistedtakes an arbitraryperiods[]array with no guard whatsoever on its values. The contract permits the dangerous path and signals nothing.Waiting defeats the purpose of the rescue. It would leave a blocklisted account's value in limbo for one to two full periods — the opposite of what a compliance rescue exists to do.
Nothing communicates a timing constraint. Not the signature, not the NatSpec, not any revert. The only way to know is to have already found this bug.
A batch rescue cannot avoid it. An account with requests spread over several periods is rescued in a single call (trigger condition #5), and such a call necessarily includes the not-yet-claimable period.
A rescuer following the obvious, documented behaviour produces the broken state.
Exploit scenario
Every role action below is legitimate and squarely within its documented privileges — this is the normal compliance workflow, not misuse.
1
Victim requests a withdrawal during period C
withdrawSharesOf[C+1][victim] > 0
2
Victim is blocklisted (compliance suspicion)
—
3
Rescuer calls rescueWithdrawFromBlocklisted(victim, safe, [C+1])
withdrawSharesOf[C+1][victim] = 0, isWithdrawClaimed[C+1][victim] = true
4
Suspicion cleared, victim unblocklisted (still period C)
—
5
Victim files a new withdrawal → targets currentPeriod()+1 = C+1
withdrawSharesOf[C+1][victim] > 0, shares burned
6
Period advances, victim calls claimWithdraw(C+1)
revert AlreadyClaimedPeriod
Step 3 is not an odd operator choice: a blocklisted user cannot claim (claimWithdraw carries notBlocklisted(_msgSender()), reverting BlocklistedAddress), so rescuing is the only mechanism available to move that pending value. Verified in test R1.
No recovery path exists:
claimWithdrawreverts forever — the flag is never cleared;a second rescue requires
isWithdrawClaimed[period][from] == false→ also reverts;the shares are already burned, so the amount cannot be re-requested.
Exact trigger conditions
The bug fires only when a rescue targets a not-yet-claimable period and a later request lands on that same period. Both edges are asserted:
1
Rescue a period that is already claimable
No bug — a new request can only target currentPeriod()+1, never a past period
2
Re-request one period later
No bug — lands on a fresh, unpoisoned period
3
Re-request via redeem()
Reproduces identically — same _requestWithdraw
4
rescueSharesFromBlocklisted (shares only)
No poisoning — never touches isWithdrawClaimed
5
Rescue passing several periods in one call
Every period is poisoned at once — damage scales from one transaction
Rows 1, 2 and 4 are included to show the finding is precisely scoped rather than over-generalised. Row 5 matters for severity: cleaning up an account with requests spread over several periods turns each of them into a permanent trap.
Recommended fix
Either option suffices:
In
_requestWithdraw, revert ifisWithdrawClaimed[period][receiver]is alreadytrue; orIn
rescueWithdrawFromBlocklisted, set the claimed flag only for periods that are already claimable — for a not-yet-claimable period, zeroingwithdrawSharesOfis enough, since the flag guards nothing once the balance is zero.
Why this was not caught earlier
test/rescue.jscovers rescuing shares and rescuing withdrawals, but never files a new withdrawal request after a rescue on the same period — the exact trigger.FirelightVault.solline coverage is 91.2% (npx hardhat coverage);_requestWithdraw's interaction with a pre-existingisWithdrawClaimedflag is among the uncovered behaviours.
This is an oversight, not an accepted design trade-off.
Impact Details
Severity: Critical — Permanent freezing of funds (impact id 17). Fallback: High — Temporary freezing of funds ≥ 24h (id 6261).
Permanent loss for the user. Shares burned, assets unclaimable forever. The victim can lose their entire remaining position in a single call — test R2 redeems the full balance into the poisoned period and ends with
balanceOf == 0andmaxWithdraw == 0.Assets permanently stranded, owned by nobody.
pendingWithdrawAssetsis incremented at step 5 and never decremented (onlyclaimWithdrawdecrements it, and it reverts). SincetotalAssets() = super.totalAssets() - pendingWithdrawAssets(:427-429), the tokens physically remain in the vault whiletotalAssets()excludes them forever: not claimable by the victim, not redistributed to anyone else. Asserted:asset.balanceOf(vault) - vault.totalAssets() >= frozenAmount, and it does not heal over subsequent periods.Reduced cover capacity.
CoverOrderAllocator._computeAvailableCapacity(CoverOrderAllocator.sol:710-714) values the vault leg asvault.totalAssetsAt(vault.currentPeriodStart())— a checkpoint oftotalAssets() = super.totalAssets() - pendingWithdrawAssets, which permanently excludes the stranded amount. The capacity computation therefore values the vault below the collateral it actually holds, for good.Asserted in test R5, on the exact quantity the allocator reads: the checkpoint returned by
totalAssetsAt(currentPeriodStart())equals the livetotalAssets()(so there is no temporal skew between the two readings), andasset.balanceOf(vault) - checkpointequals the frozen amount exactly — still short five periods later.The withdrawal path gives no warning.
maxWithdraw,maxRedeem,balanceOf,previewWithdrawandwithdrawalsOfall return healthy values, andwithdraw()succeeds and emitsWithdrawRequestnormally. The rawisWithdrawClaimedmapping is a public getter and does returntrue— but nothing on the withdrawal path consults or surfaces it: the request is accepted and the shares burned regardless. A user following the documented ERC-4626 flow has no signal. This is why it is a protocol defect, not user error.
Bounds of the claim (test C). The victim is not locked out of withdrawing in general: a later request naming a different receiver claims normally. What is permanently frozen is the amount already committed to the doomed request — shares burned, assets unclaimable, no way back. And since nothing on the withdrawal path signals the poisoning (Impact 4), the victim has no reason to name a different receiver until after the loss has already occurred.
Explicitly NOT claimed (verified, stated for accuracy): the share price is not distorted for other depositors. When the doomed request was filed, the victim's shares were burned at the same moment pendingWithdrawAssets grew, so totalAssets/totalSupply is preserved — previewDeposit returns an identical quote before and after the freeze. Remaining holders are neither diluted nor enriched; the loss falls entirely on the victim.
Scope justification
No role acts maliciously or incorrectly. Blocklist → rescue → unblocklist is the expected operational sequence of a compliance system. The victim is a legitimate user.
The rescue is forced, not optional. A blocklisted account cannot call
claimWithdraw(revertsBlocklistedAddress), so its pending value can only be moved byrescueWithdrawFromBlocklisted.Rescuing a not-yet-claimable period is the normal case. Requests target
currentPeriod() + 1, so a request stays non-claimable throughout the period in which it was filed and the next one. A rescuer reacting promptly necessarily lands in that window; nothing in the signature, NatSpec or revert conditions suggests waiting, and the function explicitly accepts anyperiods[i].It bypasses an intended constraint. The implicit invariant "an accepted withdrawal request remains claimable" is broken even though
_requestWithdrawaccepted the request, burned the shares and took the value.
References
contracts/core/FirelightVault.sol:845-876—rescueWithdrawFromBlocklisted, the write at:870contracts/core/FirelightVault.sol:994-1028—_requestWithdraw, no read of the flagcontracts/core/FirelightVault.sol:788-811—claimWithdraw, the revert at:794contracts/core/FirelightVault.sol:427-429—totalAssets()contracts/core/CoverOrderAllocator.sol:710-714—_computeAvailableCapacity
Proof of Concept
Proof of Concept
Step-by-step reproduction
Every role acts strictly within its documented privileges. This is the normal compliance workflow of the protocol, not misuse.
A user files a normal withdrawal request. During period
C, the victim callswithdraw(amount, victim, victim)._requestWithdrawburns the shares and creditswithdrawSharesOf[C+1][victim], because a request always targetscurrentPeriod() + 1.Compliance blocklists the account. The blocklister calls
addToBlocklist(victim)on suspicion. From this moment the victim can no longer callclaimWithdraw— it carriesnotBlocklisted(_msgSender())and revertsBlocklistedAddress. The pending value can now only be moved by a rescue.The rescuer moves the pending request to a safe address.
rescueWithdrawFromBlocklisted(victim, safe, [C+1])zeroeswithdrawSharesOf[C+1][victim]and creditssafe. It also setsisWithdrawClaimed[C+1][victim] = true(FirelightVault.sol:870), with no check thatC+1is already claimable — and it is not, since we are still inside periodC.The suspicion is cleared and the account is un-blocklisted.
removeFromBlocklist(victim), still within periodC.The victim files a new withdrawal request. It targets
currentPeriod() + 1 = C+1— the poisoned period._requestWithdrawnever readsisWithdrawClaimed, so the request is accepted, the shares are burned,pendingWithdrawAssetsgrows, andWithdrawRequestis emitted normally.maxWithdraw,maxRedeem,balanceOfandpreviewWithdrawall keep reporting healthy values.The period advances and the claim reverts, permanently.
claimWithdraw(C+1)hitsif (isWithdrawClaimed[period][sender]) revert AlreadyClaimedPeriod(period)(:794). There is no way out: the flag is never written back tofalseanywhere in the contract (the only two writes,:806and:870, both settrue); a second rescue requires the flag to befalseand therefore reverts too; and the shares are already burned, so the amount cannot be re-requested.
The assets stay physically in the vault while totalAssets() excludes them forever — claimable by nobody, redistributed to nobody.
Running the tests
The suite is a single self-contained file written against the project's own Hardhat fixtures (test/setup/fixtures.js). No external harness, no mocks beyond what the repository already ships. Drop it into test/ and run:
Test file — test/FL02_full_suite.js
Output — 16 passing, 0 failing
What each test establishes
POC / CONTROL — the freeze reproduces, and the identical flow without the rescue claims successfully. The defect is attributable to the rescue, not to the delayed-withdrawal mechanism.
A, B — the funds are in limbo and no admin or role path clears the flag (second rescue, re-blocklist, pause/unpause cycle all fail).
C — bounds the claim honestly: a later request naming a different receiver does claim. What is frozen is the amount already committed.
D — the doomed request is accepted silently, with
WithdrawRequestemitted normally.1, 2, 4 — the negative controls. Rescuing an already-claimable period, re-requesting one period later, and
rescueSharesFromBlocklistedalone all produce no bug. The finding is precisely scoped rather than over-generalised.3 — reproduces identically through
redeem().5 — a multi-period rescue poisons every period passed in one call, so the damage scales from a single transaction.
R1–R5 — each closes one anticipated objection, including R4 which states explicitly what is not claimed (the share price is unaffected).
Was this helpful?