Audit 02-2026 Fixes + Fund Batch 01#51
Merged
Merged
Conversation
#40) * fix: add repay timelock to prevent facilitator deposit theft (CS-I-11) Enforce a configurable delay between setRequest() and the first pull()/repay() call so that guardians and depositors can review the request before funds are moved. The timelock is a global parameter set at initialization and only settable by the owner. - Add uint40 repayTimelock to FacilityStorageData (packs with pausedUntil) - Add uint40 requestSetAt to Intent struct (packs with resolved) - Revert with RepayTimelockActive if pull/repay is called too early - Add setRepayTimelock (onlyOwner) with RepayTimelockSet event - Add repayAvailableAt view for off-chain monitoring - Add PoC test demonstrating the attack is blocked - Add FAC-11 and FAC-12 invariants for timelock enforcement * fix: restrict repay timelock to repay only, add act_setTimelock invariant action The timelock should only prevent repay(), not pull(). Move the timelock enforcement from the shared _initialRequestParameters() into the repay() function. Add act_setTimelock handler action to fuzz the timelock duration during invariant runs. * fix: add checkNotZero to setRepayTimelock and align NatSpec - Validate repayTimelock is non-zero via LibChecks.checkNotZero - Fix NatSpec: timelock only gates repay(), not pull()
|
Important Review skippedToo many files! This PR contains 185 files, which is 35 over the limit of 150. Please upgrade to a paid plan to get higher limits. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (185)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
…-12) (#41) Add minPt and minYt parameters to Request.mint() so the minter can specify minimum expected PT/YT amounts. Reverts with SlippageExceeded if authorizeMinting has been overwritten below the caller's minimums. - Add SlippageExceeded error to LibRequestErrors - Update IRequest.mint(uint128 minPt, uint128 minYt) signature - Add slippage check in Request.mint() before clearing authorization - Add PoC test demonstrating the front-run attack is blocked - Add unit and fuzz tests for slippage protection - Update existing mint() call sites to use new signature
* docs: document swap digest signer-address replay risk (CS-I-9) Add NatSpec security notes to FacilitySwap and IFacilitySwap documenting that the EIP-712 swap digest does not hash the signer address. SC wallet guardians must use implementations that bind their own address into EIP-1271 isValidSignature (e.g., Safe >= 1.3.0). * docs: remove audit reference from NatSpec
… (CS-I-6) (#43) * fix: return 0 from previewDeposit/previewMint for ERC-4626 compliance (CS-I-6) previewDeposit() and previewMint() returned calculated amounts even though deposit() and mint() always revert. Per ERC-4626, preview functions must reflect the actual outcome of the corresponding operation. Return 0 since no deposits/mints are possible. - Update previewDeposit() and previewMint() to return 0 - Update existing tests to assert 0 return values - Add PoC tests verifying full deposit/mint compliance surface * chore: remove unnecessary PoC test
…(CS-I-13) (#44) Enforce a configurable delay between the last mint/consume and setRepaid() to prevent a colluding facilitator from inflating YT supply and atomically marking the request as repaid, stealing yield from legitimate holders.
Update README state diagrams and access control table to reflect that setFund() and setRequest() are callable in any intent phase (DEPOSITING, RESOLVING, RESOLVED), not just RESOLVING as previously documented.
When mulDiv rounds a token's proportional share to zero, skip the safeTransfer call. Some ERC-20 implementations revert on zero-amount transfers which would brick claim() for small shareholders.
The deposit cap invariant incorrectly asserted totalSupply <= depositCap globally. Since the cap can be reduced below current supply via setDepositCap, the invariant should only verify that deposits cannot push supply above the cap. - Convert FAC-4 from global assertion to per-action ghost flag - Add act_updateDepositCap handler action to exercise cap reduction - Add depositExceededCap ghost checked after each successful deposit
Calling setFund or setRequest a second time with the same address deleted the reverse mapping (fundsIntent/requestsIntent) via abandonFund/abandonRequest while the intent still referenced it. This broke the uniqueness check, allowing the same fund/request to be assigned to another intent. Add early return when newFund/newRequest matches the current value.
* fix: match getIntent return values in test destructuring * build: update optimizer runs
When totalYAssets = 0 (e.g. after full liquidation), convertToShares uses the initial conversion ratio of type(uint256).max for non-zero YT amounts, causing yt.withdraw to revert instead of gracefully handling the edge case. Add an early return for zero-amount withdrawals.
_approve() previously zeroed the sibling token allowance because it passed 0 for the other token. Now reads existing allowances and only overwrites the targeted token, preserving the sibling's allowance.
_transfer did not validate the recipient, allowing tokens to be sent to address(0) without reducing totalSupply. Add checkNotZero(to) using LibChecks so transfers to the zero address revert with AddressZero().
…#54) * fix: add cancelRecovering() to prevent permanent fund lock (CS-I-4) If recovering() is called by mistake while PROCESSING and Superstate delivers the output tokens, funds become permanently stuck. Add cancelRecovering() to revert internalState back to PROCESSING. * fix: forge fmt * refactor: USCCFund implements IUSCCFund * test: register act_cancelRecovering and remove early returns
* fix(wrapped-asset): centralize InvalidDecimals in LibFundsErrors * refactor(wrapped-asset): derive decimals from underlying token
Lock the compiler to a specific version instead of relying on auto-detect with floating pragma `^0.8.20`.
Update the resolveStart comment to accurately describe it as the timestamp at which the deposit phase ends and the resolving phase begins. Fixes 3F-71
In recover() and unlock(), transfers and events incorrectly used msg.sender as the recipient. The IFund interface specifies that output assets and shares must go to order.receiver.
The cachedBalance storage variable was a defensive snapshot of the USCC balance before Superstate processing. Since commit() always leaves the fund with zero USCC (deposit sends USDC; redeem burns USCC via offchainRedeem), the cached value was always zero and the zeroFloorSub was a no-op. Removing it simplifies storage, gas, and the _state() logic.
* fix: use _setOracle helper in USCCFund.initialize Extract oracle validation (contract check, decimals check, storage set) into a private _setOracle() helper to eliminate duplication between initialize() and setOracle(). * fix: move event in internal function * fix: rollback cancelRecovering
* fix: add minBalance param to setRepaid() to prevent frontrun (Fixes 3F-79) The facilitator could frontrun setRepaid() by calling pullFunds() to drain the contract before repayment is finalized. Adding a minBalance parameter lets the owner specify the minimum expected asset balance, reverting if the balance has been drained below that threshold. * docs: update setRepaid() references to setRepaid(uint256)
The PositionManager's LTV is not a liquidation loan-to-value but a simple loan-to-value threshold — a small buffer above the target that determines how much collateral can be withdrawn from borrow positions based on debt. Renames across 31 files: - Storage field: lltv → ltv - Functions: setLltv → setLtv, checkValidLltv → checkValidLtv - Error: InvalidLltv → InvalidLtv - Event: LLTVSet → LTVSet - IBorrowPosition interface params: lltv → ltv - All NatSpec and test references updated accordingly Morpho's own marketParams.lltv and LiquidationLtvExceedsMarketLltv are kept as-is since those refer to Morpho Blue's actual liquidation LTV. Fixes 3F-80
- Remove unused `currentOrder` from USCCFund storage (stored but never read) - Replace `resolvedOrder` struct (5 slots) with minimal fields (bool + 2 uint256) - Remove redundant `delete` operations in cancel/recover/unlock (cleared by next create) - Use direct `_values` mapping access in LibTokenBalances to skip unnecessary contains() check Fixes 3F-75
… (3F-84) (#69) convertToShares used mulDiv (round down) unconditionally. When burning shares on asset decrease, rounding down means the caller surrenders fewer shares than the value extracted, leaking fractional value from LPs. Added a roundUp boolean param to convertToShares. The burn path in _settleShares now passes roundUp=true (mulDivUp), ensuring the caller surrenders at least as many shares as the value removed.
…F-85) (#68) Previously totalAssets() summed all collateral and all debt globally, then applied zeroFloorSub once. A position with bad debt (debt > collateral) would reduce the overall NAV, understating the value for healthy positions. Now each position's NAV is computed individually using zeroFloorSub, so bad-debt positions contribute 0 instead of a negative value.
* fix: accrue interest before reading Morpho market data (3F-83) Stale Morpho market data from IMorpho.market() caused debt to be systematically understated, affecting health checks, borrowing capacity, fee computation, and share settlement. For state-changing functions (withdrawCollateral, borrow, preLiquidate): call morpho.accrueInterest() before reading market data. Critically, preLiquidate now accrues interest BEFORE the health check instead of after, preventing stale data from blocking legitimate liquidations. For view functions (totalBorrowed, isHealthy, maxBorrow, availableLiquidity, availableCollateral): introduce MorphoExpectedLib, a Solady-based library that computes expected interest-accrued market balances without mutating state, mirroring Morpho Blue's _accrueInterest logic exactly. MorphoExpectedLib is fuzz-tested against actual Morpho accrueInterest output across arbitrary supply/borrow/time/fee combinations. * fix: use Market struct with SafeCastLib in MorphoBalancesLib Operate directly on the Market memory struct (uint128 fields) and use Solady SafeCastLib.toUint128() for safe casts, mirroring the original Morpho Blue MorphoBalancesLib pattern. This ensures overflow behavior matches Morpho exactly instead of silently allowing >uint128 values.
* feat(facility): share digest replay checks across swap and intent updates * fix: forge fmt * fix: apply pr comments * fix: keep _digest in caller function * fix: move digest replay check into base _checkSignatures
maxDeposit() always returned type(uint256).max and maxRedeem() returned the full wUSCC balance regardless of caller permissions. Now both functions check _DEPOSITOR_ROLE and return 0 for unauthorized accounts.
Replace all "Prime Broker" / "PB" references with "Bridge Facilitator" / "BF" across NatSpec comments, test variable names, and test comments.
Replace proportional collateral allocation with LTV-aware deposit logic. Each position now uses collateralForBorrow/borrowForCollateral to determine exact collateral needs at the target LTV, preventing deposits from exceeding the owner-configured LTV threshold. Changes: - Add collateralForBorrow() and borrowForCollateral() to IBorrowPosition - Implement both in MorphoBorrowPosition with full natspec - Add _requiredCollateral() and _maxBorrow() internal helpers - Update processDeposit to use LTV-bounded collateral allocation - Update MockBorrowPosition with realistic LTV-aware math - Add comprehensive tests for new view functions and LTV constraints - Update README to reflect LTV-enforced deposit flow Fixes 3F-114 Fixes 3F-171
Proportional withdrawals did not verify that each position's collateral withdrawal respects the storage LTV, unlike sequential withdrawals which use availableCollateral(ltv). This could leave individual positions in an unhealthy state during withdraw(). Burns are unaffected since amounts are proportional to total debt/collateral. Add checkLtv boolean to processWithdrawal — true for withdraw(), false for burn(). When true, each position's proportional collateral withdrawal is checked against availableCollateral(ltv) after repayment.
This reverts commit 538bb22.
Proportional withdrawals did not verify that each position's collateral withdrawal respects the storage LTV, unlike sequential withdrawals which use availableCollateral(ltv). This could leave individual positions in an unhealthy state during withdraw(). Burns are unaffected since amounts are proportional to total debt/collateral. Add checkLtv boolean to processWithdrawal — true for withdraw(), false for burn(). When true, each position's proportional collateral withdrawal is checked against availableCollateral(ltv) after repayment.
…(CS-GRUNT-014) (#151) When a vault was repaid but had zero principal assets remaining, convertToAssets incorrectly returned ptShares (1:1 initial conversion) instead of 0, causing burnAll to revert on the subsequent zero-balance safeTransfer. The fix distinguishes pre-repayment from post-repayment state: initial conversion (1:1 for PT) is now only used when supply is zero (div-by-zero guard) or when not yet repaid. After repayment, mulDiv correctly yields 0 for depleted vaults.
* fix: change mint slippage from minPt to maxPt to prevent PT inflation attack (CS-GRUNT-018) The mint function's first parameter was minPt (floor on deposit amount) but should be maxPt (cap on deposit amount). A consumer could front-run by increasing the PT authorization, forcing the broker to deposit more than expected for the same yield. The maxPt parameter now caps the deposit so ptMintAuth > maxPt reverts. Callers that previously passed mint(0, 0) for no protection should now pass mint(type(uint128).max, 0). * fix: bound fuzz delay to < 90 days to satisfy initialize constraint The testFuzz_setRepaid_respectsTimelock test bounded delay to [1, 90 days] but initialize requires repaymentDeadline > block.timestamp + mintToRepaidDelay. When delay == 90 days the strict inequality fails. Bound to 90 days - 1. * fix: early return on zero-authorized mint to prevent setRepaid griefing A user with zero PT/YT minting authority could repeatedly call mint() to bump lastMintTimestamp, permanently delaying setRepaid(). Now mint() returns early when both ptMintAuth and ytMintAuth are zero.
…-GRUNT-005) (#153) A malicious facilitator who is also a significant YT holder could over-repay assets just before setRepaid is called, inflating their YT redemption value at the expense of intent shareholders. The new maxBalance parameter caps the allowed balance, reverting with ExcessiveBalance if exceeded. Pass type(uint256).max for maxBalance to skip the upper bound check. Also fixes the pre-existing fuzz bound bug in testFuzz_setRepaid_respectsTimelock.
…partial (3F-200) (#156) When burn passes checkLtv=false to processWithdrawal, it assumes proportional amounts are safe because they're derived from total debt/collateral. This only holds when the withdrawal queue covers ALL whitelisted positions. If the queue is a strict subset, queue positions bear a disproportionate share of the withdrawal, potentially violating individual position LTV limits. Now checkLtv is only skipped when withdrawalQueue.length == borrowModules.length().
…n (CS-GRUNT-090) (#157)
…quidity (CS-GRUNT-089) (#160) Fee shares only redistribute ownership among suppliers; they do not change totalSupplyAssets and have no effect on available liquidity.
… non-whitelisted modules (CS-GRUNT-093) (#161)
…borrow (CS-GRUNT-092) (#163)
…UNT-019) (#165) maxWithdraw now returns convertToAssets(balanceOf(owner)) and maxRedeem returns balanceOf(owner) instead of type(uint256).max, satisfying the ERC-4626 requirement that these functions reflect the actual amount accepted for the specific owner.
…-GRUNT-085/087/097/098/099) (#166) * fix: use two sequential ceilings in _computeSeizedAssets to match Morpho's two-floor health check (CS-GRUNT-085) Morpho's _isHealthy applies two sequential floors: floor(collateral * price / SCALE) then floor(result * lltv / WAD). The old code combined them into a single floor(price * lltv), which could be 1 unit too optimistic, causing partial pre-liquidations to revert on Morpho's withdrawCollateral health check. * fix: use two sequential ceilings in _requiredCollateral to match _isHealthy's two-floor check (CS-GRUNT-099) _requiredCollateral used a single combined ceiling to invert _isHealthy's two sequential floors, causing withdraw(availableCollateral(ltv)) to intermittently revert. Split into two mulDivUp calls matching the pattern already applied in _computeSeizedAssets. * fix: simulate share round-trip in collateralForBorrow to prevent post-borrow health check revert (CS-GRUNT-098) collateralForBorrow predicted post-borrow debt as currentDebt + borrowAmount, but Morpho's borrow() converts assets to shares via toSharesUp then _isHealthy reconverts via toAssetsUp. This double rounding can inflate actual debt beyond the prediction. Now simulates the exact share round-trip to match what _isHealthy will see. * fix: compute _maxBorrow in share space to match Morpho's rounding (CS-GRUNT-087) _maxBorrow computed capacity in asset space, but Morpho's borrow() converts assets to shares via toSharesUp then _isHealthy reconverts via toAssetsUp. The compounded rounding could make borrow(maxBorrow(ltv)) revert. Now computes in share space using Morpho's own SharesMathLib: find max shares via toSharesDown, convert to assets via toAssetsDown. This directly matches Morpho's rounding with no post-hoc adjustment needed. Same fix propagates to borrowForCollateral which also delegates to _maxBorrow. * fix: use toAssetsDown in totalBorrowed to prevent repay revert (CS-GRUNT-097) totalBorrowed() used toAssetsUp which rounds up. When passed to repay(), Morpho converts back via toSharesDown — the round-trip toSharesDown(toAssetsUp(bs)) can exceed bs, causing underflow. Switching to toAssetsDown ensures toSharesDown(toAssetsDown(bs)) <= bs, so repay(totalBorrowed()) never reverts. May leave at most 1 dust share whose asset value rounds to 0.
…revent share inflation via direct Morpho collateral donation (#167)
* feat: Centrifuge Fund Integration (JAAA & ACREDX) (#78) * refactor: reorganize USCC fund files into funds/USCC/ subfolder Move USCC-specific files into dedicated USCC/ subfolders to prepare for additional fund types alongside USCCFund. * feat: vendor Centrifuge IVaultRouter interface (3F-94) Add a flattened, self-contained IVaultRouter interface with all contract-type parameters replaced by plain address for ABI compatibility. PoolId and ShareClassId UDVTs are defined inline and IBatchedMulticall is flattened in. This removes the need for the centrifuge_export/ vendor directory. * feat: CentrifugeFund * refactor: remove dead resolve(), validate output at creation, clean up _state Remove resolve() and its storage fields (hasResolvedAmounts, resolvedInput, resolvedOutput) which are unused — CentrifugeFund queries the vault directly unlike USCCFund. Validate order.output against the vault's conversion rate at creation time. Inline RECOVERING branch in _state() to avoid duplicate claimableCancel* external calls. Collapse recovering/cancelRecovering/cancelOrder into a single cancelRequest(). * fix: forge fmt * refactor: add NatSpec, use before/after pattern, simplify RECOVERING state Add NatSpec to storage struct fields, _stateHasPendingRequest, and _stateHasPendingRecover. Document requestId = 0 convention. Use before/after balance delta in unlock() and recover() to exclude dust. Simplify _state() RECOVERING branch to only check claimable amount, mirroring the PROCESSING/UNLOCKING pattern. * doc: missing comments * doc: add CentrifugeFund to README * refactor: move archived order check from _state() to state() * feat: add MAX_OUTPUT_DEVIATION slippage floor to CentrifugeFund Enforce a minimum output relative to the current exchange rate in create(), preventing callers from setting output to 0 or unreasonably low values. Move BPS constant from LibConstants to global Constants.sol. * fix: reset vault approval to 0 after commit requests * fix: report dynamic state in unlock() revert * fix: report dynamic state in recover() revert * feat: use vault's maxDeposit/maxRedeem limits in CentrifugeFund * refactor: cache redundant storage reads in CentrifugeFund * refactor: cache computed orderId instead of storage read in cancelRequest * refactor: simplify PROCESSING branch in _state() to match RECOVERING style * refactor: move unknown-order guard from _state() to state() * refactor: add constructor section and natspec, remove unused _orderId in _state() * refactor: extract repeated owner check into _checkOwner() * refactor: rename _checkOwner to _checkOrderOwner * fix: return archived order first in state * test: add CentrifugeFund test suite with 100% coverage Unit tests (73), fuzz tests (8), invariant tests (4), and factory tests (8) covering all CentrifugeFund operations including partial fills, slippage guard, cancel request, and state machine transitions. * fix: remove upper bound check on slippage guard in create() Allow order.output > expectedOutput (optimistic pricing). Only reject when output deviates too far below the current rate. * fix: forge fmt * fix: guard cancelRequest() against partial fills Revert with PendingClaimableAssets when claimable partial-fill assets exist (maxMint for deposits, maxWithdraw for redeems), forcing the operator to unlock() before cancelling to prevent stuck shares. * fix: add isPermissioned guard to commit() Prevents opaque vault reverts if permission is revoked between create() and commit(), matching the existing check in create(). * test: add integration tests * build: link secret * test: expand CentrifugeFund coverage with fork and fuzz tests Fork: add redeem cancel lifecycle, partial redeem fill, and slippage guard tests with real on-chain conversion rates. Fuzz: add non-1:1 rate tests for slippage guard and totalAssets. Fix invariant to use vault.convertToAssets instead of hardcoded 1:1 assumption. * fix: address PR review comments on CentrifugeFund - Reset share token allowance after wrappedShare.mint in unlock() and recover() for safety - Add _PENDING_REQUEST constant for readability instead of raw 0 - Change MAX_OUTPUT_DEVIATION from 1% to 5% for consistency with USCCFund - Update all test thresholds to match 5% slippage guard * feat: Pareto Fund Integration (FalconX Credit Vault) (#110) * feat: ParetoFund * feat: add resolve() to ParetoFund for operator output override Allow operator/owner to override order input/output after commit via resolve(), following the USCCFund pattern. The deposit state transition check in _state() uses the resolved output when available. * fix: use InvalidUnderlyingAsset error for wrappedShare mismatch Replace misused InvalidReceiver with a dedicated InvalidUnderlyingAsset error when wrappedShare.underlying() does not match the expected share token during initialize(). Applies to CentrifugeFund and ParetoFund. * refactor: rename NotAllowedToOperateWithVault to NotAllowedByFund * style: align IParetoFund and ICentrifugeFund interfaces with IUSCCFund conventions Add full NatSpec, section banners, and OPERATOR_ROLE/DEPOSITOR_ROLE view getters to IParetoFund and ICentrifugeFund for consistency. * test: add unit/invariant tests * fix: forge fmt * test: integration tests * docs: add ParetoFund to README * fix: correct totalAssets NatSpec and remove unused resolvedInput storage - Fix virtualPrice decimal description (underlying decimals, not 18) - Remove resolvedInput from ParetoFundStorage (no RECOVERING state) * fix: use before/after balance pattern in unlock() for REDEEM Use actual received balance instead of trusting withdrawsRequests() amount, which may differ due to fees, rounding, or partial fills. * feat: add isWalletAllowed check in commit() and test for revoked allowance * fix: store deposit-received amount in commit() and use it in unlock() Prevents unlock() from wrapping the entire AA tranche balance (which could include airdrops or accidental transfers) by capturing only the tokens actually received from depositAA via a before/after balance pattern, storing the result in depositReceived, and using it in both _state() and unlock(). * feat: add slippage guard on create() in ParetoFund Reject orders whose output deviates more than 5% below the current virtualPrice rate, matching the existing CentrifugeFund behaviour. * fix: forge fmt * test: add missing coverage for ParetoFund unlock and resolve edge cases Add 6 tests covering PR review feedback: - claimWithdrawRequest returning a different amount than withdrawsRequests - resolve() with input=0 and output=0 - accidental tokens sent to the fund not being swept on unlock * style: run forge fmt * fix(test): pin fork tests to block 24570780 to prevent mainnet state drift * fix: use `depositDuringEpoch` (#119) * fix: Fund Batch [01] (#125) * fix: make OPERATOR_ROLE and DEPOSITOR_ROLE internal in CentrifugeFund and ParetoFund (#123) * feat: add `MorphoFlashLoanRequest` facilitator contract (#118) * feat: add MorphoFlashLoanRequest facilitator contract Introduces a facilitator contract that uses Morpho flash loans to temporarily act as a request on a facility intent. It atomically sets itself as the request, pulls the flash-loaned amount into the facility, executes caller-specified operations, repays back, unsets itself, and returns the flash loan to Morpho. * style: run forge fmt on MorphoFlashLoanRequest * fix: set rawDebt to full balance to prevent dust griefing Set rawDebt to the contract's full token balance at callback start (flash loan amount + any pre-existing dust) instead of just the flash loan amount. This prevents an attacker from sending 1 wei of dust to revert the entire flash loan cycle via BalanceExceedsDebt(). * test: cover remaining branch paths in MorphoFlashLoanRequest Add 3 tests exercising revert branches that require non-zero rawDebt in transient storage, bringing branch coverage from 50% to 100%. * feat: add MorphoFlashLoanRequestFactory contract * style: rename storage variable from `s` to `$` --------- Co-authored-by: maximebrugel <maxime.brugel@gmail.com> * fix: handle fulfilled shares stuck after `cancelRequest` races with `approveDeposits` (#133) * fix: handle fulfilled shares stuck after cancelRequest races with approveDeposits When cancelRequest() passes its maxMint==0 guard between Centrifuge's multi-step fulfillment (approveDeposits → issueShares → notifyDeposit), the deposit can be partially or fully fulfilled despite the cancel. The RECOVERING state previously only checked claimableCancelDepositRequest, making fulfilled shares permanently stuck. Update _state() to check maxMint/maxWithdraw first when in RECOVERING, returning UNLOCKING so fulfilled shares can be claimed. Update unlock() to transition back to RECOVERING (not PROCESSING) when cancel assets still need claiming. Update invariant model to match new state logic. * test: add race condition handler actions and redeem partial fulfillment test Add fulfillDepositAndCancelDeposit / fulfillRedeemAndCancelRedeem handler actions so the fuzzer can reach the race condition path where the Centrifuge pool fulfills both the request and its cancellation in the same epoch. Also add test_Unlock_RedeemPartialFulfilledDuringRecovery mirroring the existing deposit-side test. * fix: pack `CentrifugeFundStorage` struct to save one storage slot (#134) Move `internalState` (1-byte enum) next to `vault` (20-byte address) so they share slot 0, reducing the struct from 6 slots to 5 and saving ~2,100 gas on cold SLOAD when both fields are accessed together. * fix: revert on reused order ID to prevent ambiguous state (#135) Add `OrderAlreadyExists` guard in `create()` for CentrifugeFund, ParetoFund, and USCCFund to reject orders whose computed ID already exists in `endedOrders`, preventing `state()` from returning a stale ENDED result for a newly created order. * fix: check WrappedShare is permissioned in `create()` and `commit()` (#137) Without this check, orders proceed through ACCEPTED → PROCESSING before reverting at `unlock()` when WrappedAsset lacks share token permissions, locking assets recoverable only via async `cancelRequest → recover`. * fix: add `forceEnd` to CentrifugeFund to unblock griefed orders (#136) * feat: add `forceEnd` to CentrifugeFund to unblock griefed orders An attacker can call `vault.requestDeposit(1, fundAddress, attacker)` to inflate `pendingDepositRequest(0, fundAddress)`, preventing the fund from transitioning out of PROCESSING after a legitimate fill is claimed. `forceEnd` lets the operator or owner force-end a stuck order from PROCESSING or RECOVERING state, bypassing the vault's pending counters. * fix: guard `forceEnd` against stranding claimable assets * fix: return 0 from `maxDeposit` and `maxRedeem` for non-depositors (#138) * fix: use `lastWithdrawRequest` in ParetoFund to unblock REDEEM on apr0 vaults (#139) When IdleCreditVault has unscaledApr == 0, requestWithdraw routes amounts through _requestWithdrawApr0 and never increments withdrawsRequests[user]. ParetoFund._state() relied on withdrawsRequests > 0 to detect claimable withdrawals, causing REDEEM orders to be permanently stuck in PROCESSING. Switch to lastWithdrawRequest > 0 as the pending-withdrawal signal, which is set on both the normal and apr0 paths and reset to 0 after claim. * fix: remove unnecessary tranche token approvals in `ParetoFund.commit()` (#140) IdleCDO's `requestWithdraw` burns tranche tokens directly via `IdleCDOTranche.burn()`, which does not use `transferFrom` and therefore requires no approval. Remove the redundant approve/reset calls and fix the mock to match the real CDO behavior. * fix: allow `ParetoFund.cancel()` in PENDING state per IFund spec (#141) * fix: remove misleading `newOrderId` from `OrderResolved` event (#142) `resolve()` emitted a `newOrderId` computed from in-memory order mutations, but `currentOrderId` was never updated — so the emitted ID didn't correspond to any on-chain state. Remove the parameter and the dead `order.input`/`order.output` mutations that only existed to compute it. * fix: use `mulDiv` consistently in ParetoFund slippage guard (#143) * fix: replace literal `0` with `_PENDING_REQUEST` constant in CentrifugeFund (#144) * fix: pass `account` instead of `address(this)` in `maxDeposit` and `maxRedeem` (#145) * fix: pass `account` instead of `address(this)` in `maxDeposit` and `maxRedeem` * docs: add @dev comments to `maxDeposit` and `maxRedeem` explaining vault permissioning * fix: CentrifugeFund.recover() and unlock() return PROCESSING per IFund spec (#146) * fix: return `PROCESSING` instead of `RECOVERING` from `recover()` and `unlock()` per IFund spec `recover()` and `unlock()` returned `RECOVERING` for partial operations, violating the IFund interface which specifies `PROCESSING`. Decoupled the internal state (stays `RECOVERING` for correct state-machine transitions) from the return value (now `PROCESSING` per spec). * fix: update stale assertion messages from "still recovering" to "still processing" * fix: revert on instant withdrawals in ParetoFund to prevent stuck orders (#148) When the CDO silently routes a withdrawal to the instant path, `withdrawsRequests` doesn't increase and the order gets permanently stuck in PROCESSING. Check before/after values and revert with `InstantWithdrawDetected` if the request wasn't registered. * refactor: extract `_revertIfUnclaimedFills` helper in CentrifugeFund (#149) Deduplicate the identical claimable-fills guard from `cancelRequest()` and `forceEnd()` into a single internal helper that also encapsulates the `_stateHasPendingRequest` bypass for polluted claimable amounts. * feat: `MorphoFlashLoanRequest` script (#150) * feat: replace Operation[] loop with whitelisted script delegatecall Replace the generic Operation[] loop in MorphoFlashLoanRequest with a delegatecall to owner-whitelisted scripts, enabling intermediate return values (e.g. variable share amounts from unlock) to flow between chained facility calls. - Add `scripts` mapping to ERC-7201 storage, `setScript`/`isScript` management - Change `execute()` signature to accept `(script, scriptPayload)` instead of `Operation[]` - Delegatecall whitelisted script in callback (preserves FACILITATOR_ROLE via msg.sender) - Add SyncDeposit script: create → commit → unlock → depositManager with computed share amounts - Update all tests to new signature; add script management and SyncDeposit integration tests * feat: add in-script balance-diff tracking to SyncWithdrawal Derive redeemAmount on-chain via shareToken balance diff before/after withdrawManager, mirroring SyncDeposit's pattern. This couples the withdrawal and redeem steps so the script provides value over separate facilitator calls. * feat: separate owner and executor roles in MorphoFlashLoanRequest (#155) Migrate from Ownable to OwnableRoles so the address that whitelists scripts (owner) is distinct from the address that executes flash loans (EXECUTOR_ROLE). Update factory to accept executor param and add tests for the new access control boundaries. * fix: CentrifugeFund `forceEnd()` deadlock and claimable fills bypass [CS-GRUNTFUND-17/18] (#168) * fix: add syncEndedOrder() to unblock intents after CentrifugeFund.forceEnd() When forceEnd() is called directly on the fund, the Facility's Intent retains a stale order (order.owner != address(0)), permanently blocking resolve(), setFund(), and create(). Add syncEndedOrder() in LibIntent that queries IFund.state(order) and clears the stale order+fund binding when the fund reports ENDED. Auto-sync in resolve() and setFund() (before the same-fund early return) so recovery requires no extra steps. * fix: check claimable fills unconditionally in forceEnd() forceEnd() delegated to _revertIfUnclaimedFills() which skips the maxMint/maxWithdraw check when a pending request exists. An attacker could submit a dust requestDeposit via the vault to create a pending request, bypassing the guard and force-ending an order with real claimable assets still present. Replace with direct unconditional checks. * fix: use `lastWithdrawRequest` in ParetoFund instant-withdrawal guard to unblock apr0 (#169) The instant-withdrawal guard in commit() compared withdrawsRequests before/after requestWithdraw(), but on the apr0 path withdrawsRequests is never set — only lastWithdrawRequest is. This caused commit() to revert with InstantWithdrawDetected for all apr0 vaults. Switch the guard to lastWithdrawRequest, which is written on both normal and apr0 paths but unchanged on the instant path. Also hardens test coverage: apr0 lifecycle and transition tests pin the bug surface, instant-withdraw test asserts clean rollback invariants, consecutive-redeem test verifies the guard across multiple cycles, and fuzz accounting uses totalClaimable. * fix: optimize ParetoFundStorage struct packing (#170) Pack `internalState` and `hasResolvedAmounts` into the same storage slot as `vault`, saving one full 32-byte slot and ~2,100 gas per cold access in functions that read these fields together. * fix: clear rawDebt at end of onMorphoFlashLoan [CS-GRUNTFUND-021] (#171) * fix: clear rawDebt at end of onMorphoFlashLoan so isRepaid() is unambiguous * test: restore repay BalanceExceedsDebt test using mock script during callback * test: restore isRepaid BalanceExceedsDebt test using mock script during callback * fix: mint enough tokens in isRepaid BalanceExceedsDebt test to actually hit _debt() guard * fix: replace Solady `SafeTransferLib.balanceOf` with `IERC20.balanceOf` (#172) * fix: replace Solady SafeTransferLib.balanceOf with IERC20.balanceOf SafeTransferLib.balanceOf silently returns 0 on call failure, masking bugs. IERC20.balanceOf properly reverts, matching the pattern already used in CentrifugeFund and ParetoFund. * fix: replace Solady SafeTransferLib.balanceOf with IERC20.balanceOf in MorphoFlashLoanRequest and Request --------- Co-authored-by: Maxence Raballand <dev@maxencerb.com> --------- Co-authored-by: maxencerb <dev@maxencerb.com>
… via consume() callback (CS-GRUNT-105) (#173)
maxencerb
approved these changes
Apr 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ChainSecurity Audit Response & Fixes
ChainSecurity Findings (Section 7 — Open Findings)
High Severity (1)
Medium Severity (14)
setFundwithdrawManagerLow Severity (39)
resolveStartsetFundandsetRequesttotalAssetsunchangedTokenControllerreverts on self-transfersmsg.senderinstead oforder.receivertotalAssetsreports global wUSCC supply instead of per-fund holdingstotalAssets()missing Chainlink price freshness check_settleSharesrounds down shares burnedaddBorrowModuleandremoveBorrowModuleskip fee accountingrepaymentDeadlinenot validatedsetRequestcan bind the same request to multiple intentsauthorizeMintingto double-mintOpen Questions (Section 8)
Informational Findings (Section 9)
addBorrowModulelacks validationsetMaxRebalanceLoss_intialPmParametersvariableFacilityLP.claim()cachedBalancein USCCFund logicpreLiquidatecreatein IFundTokenController._transfer()MorphoBorrowPosition.initialize()LibOrder.toId()Notes (Section 10)
Additional Changes
Changes not directly tied to a ChainSecurity finding, identified during remediation and internal review.
revertDepositto FacilityCOMPLIANCE_ROLEandrevertDeposit()to allow reverting deposits from sanctioned wallets without closing the intentRebalancedevent fromPositionManager.rebalance()DeadlineExpirederroraddress(0)) since removal is safe; adds expired signature errormaxDeposit/maxRedeemfor non-depositorsmaxDeposit()andmaxRedeem()return 0 for accounts without depositor roleUSCCFund.commit()^0.8.22unchecked { ++i }patterns (redundant since Solidity 0.8.22)lltvtoltvin PositionManagerlltvtoltvthroughout PositionManager — it's a target LTV, not a liquidation LTVaddress(0)safeApprovevssafeApproveWithRetryinconsistencysafeApprovewithsafeApproveWithRetryfor non-zero amounts (handles USDT-style tokens)totalAssetslogicLibView.totalAssets()invariant_repayTimelockEnforcedsetRequestshort-circuiting on unchanged addressmaxPtin setRepaid tests to match updated mint signaturemint(0, 0)— broken after #152 changed the first param fromminPttomaxPtChainSecurity Re-Audit Findings (03/20)
Open Findings (Section 7)
collateralForBorrowcan fail the health checktotalBorrowed()can revert due to rounding mismatchavailableCollateralcan fail the health checkmaxBorrowreturns an unborrowable amounttotalAssetsunchanged (partially corrected)Informational Findings (Section 9)
setSupplyQueueremoveBorrowModuleZeroSharesrevert in_settleShares