Asymmetry between deposit and redeem (pair: deposit/redeem receiver validation). deposit rejects receiver == address(this) at src/BaskVault.sol:625, but redeem only rejects the zero address.

With receiver = address(this), every direct leg goes through pay(token, vault, leg): the token transfer is a self-transfer, the vault balance does not fall, the exact-debit check at line 741 fails, _tryPay returns false, and line 724-725 credit owed[vault][token] and totalOwed[token]. claim reads owed[msg.sender], and the vault never calls claim, so this debt is unclearable forever.

Consequences: (1) the redeemer's tokens are locked in the vault for good (self-harm), and (2) removeRetired at line 368 requires totalOwed[token] == 0, so any unprivileged share holder can, with a dust redemption, make a token permanently unremovable and therefore never relistable, contradicting the brief's 'its token may then be relisted'.

Unprivileged amplifier: any holder of dust shares; no role required. Minimal fix preserving design: mirror deposit and reject receiver == address(this) in redeem (and to == address(this) in claim). Reproduced in test/scratch/Seams.t.sol::testRedeemToVaultLocksTotalOwedAndBlocksRemoval.

Trust-gap seam (access x asymmetry): minAmountsOut is bound to the asset's position in tokens[], but removeRetired (callable by anyone the moment a retired asset has managed == 0 and totalOwed == 0) swaps the last token into the removed slot at src/BaskVault.sol:375 and pops.

A redeem transaction built from previewRedeem/assetTokens before that call executes with its minimums shifted: the moved token is compared against the minimum the caller set for a different slot (often 0), and the caller's intended minimum for it falls past the array end and is ignored. Combined with recognizeLoss (also anyone, once a record is 7 days old) in the same block, an unprivileged actor can make a redeemer accept a write-down their minimum was meant to reject.

In the opposite direction the shift makes the redeem revert, a one-transaction DoS.

Impact is bounded: the redeemer receives the honest post-loss pro-rata amount, but the only slippage control redeem offers is defeated by a public call. Reproduced in test/scratch/Seams.t.sol::testRemoveRetiredFrontRunDefeatsMinAmountsOut.

A fix keeping the index-based interface could key minimums to token addresses or require minAmountsOut.length == tokens.length; the brief's 'missing entry = 0' rule would need to be revisited for that, so this is a scope decision for the author.

Trust-gap seam (access x economics, race amplifier). Resync raises managed[token] by balanceOf - managed - totalOwed, instantly raising NAV for every share. The proposal and its executable time are public (Proposed event, pendingProposals), and until execution deposits are priced on a NAV that excludes the surplus.

An unprivileged depositor who enters in the block before execute(id) is minted shares at the pre-resync NAV and immediately owns a pro-rata claim on the surplus, diluting the holders who were exposed when the surplus arrived. With no fee recipient set the capture is pure; with the 0.5% deposit and redeem fees it is profitable whenever surplus / NAV exceeds about 1%.

This is the ordinary MEV cost of a timelocked, publicly announced accounting step and the brief requires resync as specified, so this is reported as a documented trust assumption rather than a defect in the guards: the owner should expect resync to be sandwiched and could, within the existing design, pause deposits (owner or guardian, at once) in the blocks around execution. Reproduced in test/scratch/Seams.t.sol::testResyncFrontRunCapturesSurplus.

The brief says 'Anyone may remove a retired asset with managed and totalOwed 0; its token may then be relisted.' The code implements that condition literally, but the condition can never be met for a normal Stock Token once it has received a single deposit. Every redemption computes leg = min(managed, available) * net / totalSupplyBeforeBurn (line 713) with net < totalSupply, because the first deposit mints 1e15 shares to address(0xdEaD) (line 659) that nobody can ever burn.

So each redemption removes strictly less than the whole managed balance and managed converges on a dust residual (managed * 1e15 / totalSupply, rounded) that no path can zero: redeem cannot (net < supply), recognizeLoss only lowers managed by an actual shortfall (line 832-834) and a healthy token is never short because its balance equals managed, and Resync only raises managed.

The result is that after retirement the asset keeps its slot in tokens forever, the retire -> remove -> relist lifecycle promised by the brief is dead code in practice, and maxAssets capacity is permanently consumed by every retired asset. Only an asset that never received a deposit, or one whose entire custody was confiscated and recognised as a loss, can be removed. The scratch test test/scratch/RemoveRetired.t.sol::testRemoveRetiredUnreachableAfterFullExit reproduces this.

deposit rejects the vault as receiver (line 625) but redeem only rejects address(0). With receiver = address(this), the direct path calls pay(token, vault, leg): the token transfers to itself, the vault balance does not fall, and the exact-debit check at line 741 reverts, so _tryPay returns false and line 724-725 credit owed[vault][token] and totalOwed[token].

The vault never calls claim on itself (claim is keyed on msg.sender and pay is the only self-call), so that debt can never be paid and totalOwed[token] can never return to 0. Those tokens are also excluded from available forever (line 794) and from Resync (line 442), so they are stranded inside the contract. The caller only loses a dust leg: with managed 10e18 and supply 1000e18, 1000 wei of BASK is enough to produce a nonzero leg for every asset in one call.

Combined with finding 1 this closes the last remaining route to removeRetired (an asset whose custody was fully lost and recognised), and it strands a nonzero amount of every listed Stock Token with no owner. Reproduced by test/scratch/RemoveRetired.t.sol::testRedeemToVaultStrandsOwedAndBlocksRemoval.

Boundary (sentinel receiver). deposit() rejects receiver == address(this) (line 625) but redeem() only rejects address(0). With receiver == address(this) every direct leg goes through pay(token, vault, leg): the token moves vault->vault, the balance does not change, so beforeBalance - afterBalance != amount (line 741) reverts and the leg is written to owed[address(this)][token] and totalOwed[token] (lines 724-725).

The vault cannot call claim() on itself, so that debt is never cleared: totalOwed[token] stays nonzero for good, the tokens are excluded from available in every later redeem, deposit health check and resync, and removeRetired(token) (line 368, requires totalOwed == 0) reverts forever, so the token can never be relisted and its slot is consumed against maxAssets.

The redeemer burns shares for nothing (self-harm); no other holder loses because managed is reduced by the same leg. Any share holder can do this with a dust redemption, so it is a cheap, irreversible griefing of the remove/relist path for every held asset (a holder who redeems while a token is blocked and never claims achieves the same, so the design already tolerates unclaimed debt; this variant is merely deterministic and unrecoverable).

Minimal fix within the brief: treat receiver == address(this) like address(0) in redeem (the brief's 'receiver not the vault' already exists for deposit).

redeem compares each leg to minAmountsOut[i] by position in tokens. removeRetired, callable by anyone, swaps the last asset into the removed slot and pops. A redeem built from previewRedeem (or allAssets) before the removal lands has its minimum for the moved asset applied to the removed slot's old index (where the caller put 0 for the empty retired asset) and its real minimum is beyond the new length and ignored.

The slippage guard the spec requires ("require leg >= minAmountsOut[i]") is silently dropped for the moved asset. Harm needs the moved asset's leg to actually fall before the redeem lands (a shortfall or a recognizeLoss on it), so it is a narrow but concrete loss of a stated guarantee.

Fix options that keep the spec: reject a minAmountsOut array whose length is nonzero and differs from tokens.length, or avoid reordering by filling the removed slot with the last element only when the caller-supplied minima cannot be stale (e.g. keep order and compact lazily). A redeem built with a full-length array would then revert instead of running with misaligned minima.

When totalSupply is 0 the share formula uses gross = value regardless of nav. totalSupply is 0 only before the first deposit, but managed can already be nonzero then: after listing (genesis or later) tokens can be sent to the vault and the owner can execute a Kind.Resync, which sets managed from balanceOf. The first depositor then receives shares equal only to their own value yet, through redeem's pro rata of managed, owns essentially the whole pre-existing balance.

The spec text ("gross = v if totalSupply is 0") describes this formula, so the defect is the combination with Resync being executable before any deposit. Whoever sent those tokens (a seed from the owner, a transfer by mistake) loses them to the first depositor. Minimal fix preserving the spec: revert Resync execution while totalSupply is 0 (or treat nav > 0 with supply 0 as ZeroNAV-style invalid), so the first deposit always starts from an empty basket.

Every deposit, even a single-token one, reads the balance of every unretired asset, prices every asset with managed > 0 (feed call, pause call, pool observe and quote feed when a pool is set) in _depositState, and then runs a second full loop over tokens that reads assets[token].retired and deletes two deficit slots per asset (about 8.6k gas per asset cold, 2.2M at 250 assets).

With MockFeed/MockPool, which cost a few thousand gas each, a cold single-token deposit measures 12,332,173 gas at 250 assets without pools and 19,065,770 with pools. A Chainlink proxy+aggregator read costs roughly 20-30k and a Uniswap v3 observe roughly 30-60k, so at 250 pooled assets a deposit will be in the 28-40M range and at the allowed maximum of 350 assets higher still, above a 28-32M block limit. Deposits then stop for everyone while redeem keeps working.

This follows from the spec's per-deposit requirement to validate every held price, so the owner must treat the practical asset count as well below maxAssets. The second loop can be made constant-gas without changing semantics (a deficit epoch bumped on deposit and compared in flagDeficit/recognizeLoss), which recovers about 2.2M at 250 assets and 3M at 350.

deposit rejects receiver == address(this) (line 625) but redeem only rejects address(0). With receiver == address(this) each direct leg goes through pay(token, vault, leg): the token performs a self-transfer, the vault balance does not fall, the exact-debit check at line 741 reverts, _tryPay returns false and lines 724-725 credit owed[address(this)][token] and totalOwed[token]. claim is keyed on msg.sender and the vault never calls claim, so that debt is unclearable.

Consequences: the redeemer's tokens are stranded inside the vault (self-harm), those tokens are excluded from available for every later redeem, deposit health check and Resync (line 794), and removeRetired at line 368 requires totalOwed[token] == 0, so any share holder can, with a dust redemption, make a Stock Token permanently unremovable and therefore never relistable. Any holder of dust shares can do this; no role is needed.

Merged from three specialist reports (flow, permissions, math) that describe the same root cause. Minimal fix within the brief's wording for deposit: treat receiver == address(this) like address(0) in redeem.

Every redemption removes leg = min(managed, available) * net / totalSupplyBeforeBurn (line 713) with net < totalSupply, because the first deposit mints 1e15 shares to address(0xdEaD) (line 659) that nobody can burn.

After every holder exits, managed[token] is left at ceil(managed * 1e15 / supply) > 0. recognizeLoss only lowers managed by an actual shortfall (lines 832-834) and a healthy token is never short because its balance equals managed; Resync only raises managed; nothing else writes managed. So for a normal Stock Token the condition managed == 0 at line 368 can only be met by an asset that never received a deposit or whose custody was confiscated and recognised.

The retire -> remove -> relist lifecycle the brief promises is unreachable in practice, every retired asset keeps its slot against maxAssets, keeps counting in managedAssetCount against directLimit, and keeps costing a balance read in every redeem. The implementation matches the brief's literal condition; the brief's lifecycle is what does not hold, so this is a scope decision for the author.

redeem compares each leg to minAmountsOut[i] by position in tokens. removeRetired, callable by anyone the moment a retired asset has managed == 0 and totalOwed == 0, swaps the last asset into the removed slot (line 375) and pops.

A redeem built from previewRedeem or assetTokens before that call executes with its minimum for the moved asset applied to the removed slot's old index (where the caller put 0 for the empty retired asset) and its real minimum beyond the new length and ignored.

The only slippage guard redeem offers is defeated by a public call; harm needs the moved asset's leg to actually fall before the redeem lands (a shortfall or a recognizeLoss on it, which anyone may trigger in the same block once a record is 7 days old). In the opposite direction the shift makes the redeem revert, a one-transaction DoS. Merged from the permissions and economics reports.

Its precondition is narrow because of finding 2 (removeRetired needs managed == 0). A fix that keeps the brief's positional interface could reject a nonzero-length minAmountsOut whose length differs from tokens.length; the brief's 'missing entry = 0' rule would then need a scope decision.

NAV for deposits is the sum of value(managed), so tokens held beyond managed (custody returned after a recognizeLoss, an airdrop, a split, a donation) are worth 0 to NAV until the owner executes a Resync, which adds the whole surplus in one step. The proposal is public (Proposed event, pendingProposals) and executable only 2 to 9 days after it is proposed, so the step-up is announced in advance.

Any depositor who enters at the pre-resync NAV and redeems after execution takes a pro-rata slice of the surplus, and redeem pays pro rata of managed with no price so the gain is realised at once; existing holders are diluted by that amount. The 0.5% deposit and redeem fees (only if a fee recipient is set) are the whole cost.

This is the ordinary cost of a timelocked, publicly announced accounting step and the brief specifies Resync exactly this way, so it is reported as a documented economic property rather than a guard defect: the owner or guardian should pauseDeposits() from proposal until the Resync has executed. Making execution of Kind.Resync require depositsPaused would be a new safeguard and is a scope decision.

Merged from the economics (medium) and permissions (low) reports; severity set to low because the mitigation exists inside the agreed design and no guard is bypassed.

Every deposit reads the balance of every unretired asset, prices every asset with managed > 0 (feed call, pause call, pool observe and quote feed when a pool is set) in _depositState, then runs a second loop over all tokens that reads assets[token].retired and deletes two deficit slots per asset: about 8.6k gas per asset cold, 2.2M at 250 assets, 3M at the 350 maximum.

With MockFeed/MockPool a cold single-token deposit costs 12,337,284 gas at 250 pool-less assets and 19,281,131 with pools. Real Chainlink-style proxy + aggregator reads and Uniswap v3 observe calls cost several times the mocks, so at the configured maximum a deposit is expected to approach or exceed a 28-32M block limit; deposits would then stop for everyone while redeem keeps working.

The per-deposit validation of every held price is what the brief requires, so the operational conclusion is that the practical asset count must stay well below maxAssets; the second loop can be made constant-gas without changing semantics (a deficit epoch bumped on deposit and compared in flagDeficit/recognizeLoss), which recovers about 2.2M at 250 assets.