ApxUSD security record

In the `LinearVestV0` contract, `pullVestedYield()` only resets `fullyVestedAmount` and `lastTransferTimestamp`; it does not update `vestingAmount`. After a pull, the contract can hold less than `vestingAmount` while still reporting the original amount as vesting. When the admin later extends the vesting period via `setVestingPeriod()`, the schedule becomes active again (period end moves to the future). When time passes, `vestedAmount()` becomes greater than zero because `newlyVestedAmount()` uses the stale `vestingAmount` and the new period. The next `pullVestedYield()` attempts to transfer more than the contract holds and reverts. The contract can become insolvent from a pull followed by a period extension. Vested yield is pulled when users withdraw `ApyUSD` for `ApxUSD`, so this revert blocks all user withdrawals until the accounting is corrected. Because `totalAssets()` is inflated by stale `vestingAmount`, ERC‑4626 `deposit()`/`mint()` mints fewer shares than a fair price, so new depositors receive less than expected. The shortfall accrues to existing shareholders (share price is overstated), making the mispricing an actual execution loss for depositors, not just a preview/UI issue. When `vestingAmount` is inflated, `ApyUSDRateView.apy()` and `annualizedYield()` return overstated values because they rely on `unvestedAmount()`, which uses the stale `vestingAmount`. **Proof of Concept:** 1. Yield is deposited into the vesting contract. 2. Vesting period passes. 3. Yield is fully pulled; `vestingAmount` is not updated and remains equal to the original deposit; contract balance is zero. 4. The admin extends the vesting period via `setVestingPeriod(newPeriod)`; there is no checkpointing and the period end moves to the future, so the schedule appears active again. 5. When time passes, `vestedAmount()` becomes greater than zero (newly vested amount is computed using the stale `vestingAmount` and the new period). 6. A user attempts to withdraw `ApyUSD` for `ApxUSD`; the vault calls `pullVestedYield()`, which reverts on transfer due to insufficient balance. Withdrawals are blocked for all users.

The `RedemptionPoolV0` contract requires that its `asset` and `reserveAsset` have identical decimals and reverts if they do not. This assumes an 18-decimal format and prevents deploying the pool with common reserve assets like `USDC` (6 decimals) alongside `ApxUSD` (18 decimals). ```javascript constructor(address initialAuthority, ERC20Burnable asset_, IERC20 reserveAsset_) AccessManaged(initialAuthority) { // audit note: Requirement that asset and reserveAsset have identical decimals if (IERC20Metadata(address(asset_)).decimals() != IERC20Metadata(address(reserveAsset_)).decimals()) { revert DecimalsMismatch( IERC20Metadata(address(asset_)).decimals(), IERC20Metadata(address(reserveAsset_)).decimals() ); } asset = asset_; reserveAsset = reserveAsset_; exchangeRate = 1e18; } ```

`ERC20DenyListUpgradable` defines its internal init function using `initializer` instead of the `onlyInitializing` modifier. In OpenZeppelin Initializable (v5), the initializer modifier prevents re-entrancy. When the top-level `initialize()` function runs, it sets `_initializing` to `true`. When the inner `__ERC20DenyListedUpgradable_init()` is called, the initializer modifier check fails, causing the transaction to revert, preventing the `ApxUSD` and `ApyUSD` contracts from being initialized. ``` // audit note: initializer is used inside a nested initializer function function __ERC20DenyListedUpgradable_init(IAddressList initialDenyList) internal initializer { if (address(initialDenyList) == address(0)) revert InvalidAddress("initialDenyList"); DenyListStorage storage $ = _getDenyListStorage(); $._denyList = initialDenyList; } ```

In `ERC20DenyListUpgradable._setDenyList()`, the `DenyListUpdated` event emits the new address twice instead of (old, new). The state is updated before the event reads the storage value. ``` function _setDenyList(IAddressList newDenyList) internal { DenyListStorage storage $ = _getDenyListStorage(); $._denyList = newDenyList; emit DenyListUpdated(address($._denyList), address(newDenyList)); } ```

`AddressList.add()` and `AddressList.remove()` call `EnumerableSet.add()` and `EnumerableSet.remove()`, which return `true` when the set was modified and `false` when it was not (e.g. `add` when the address is already in the list, `remove` when it is not). ``` function add(address user) external override restricted { if (user == address(0)) revert InvalidAddress("user"); _addresses.add(user); // audit note: This call returns a boolean emit Added(user); } ``` The contract does not check these return values and always emits `Added(user)` or `Removed(user)`. Off-chain indexers or UIs that rely on these events may incorrectly assume a state change occurred when none did.:

In `ApyUSD._withdraw()`, the fee is always deducted from the user's shares (burning shares for `assets + fee`). However, the fee transfer to `feeWallet` is conditional on `fee > 0 && feeRecipient != address(0) && feeRecipient != address(this)`. If `unlockingFee > 0` but `feeWallet` has not been set (defaults to `address(0)`), the fee is still charged from the user but remains in the vault, effectively donating it to remaining ApyUSD holders. This is not transparent and could silently penalize early withdrawers if the admin sets a fee before configuring the fee wallet.

CommitToken and UnlockToken implement an `ERC-7540`-style async redeem vault and claim compliance via `supportsInterface` (returning `true` for `IERC7540Redeem` and `IERC7540Operator`). However, the implementation deviates from the spec's MUST requirements in ways that may break composability with `ERC-7540`-compliant integrations. These are intentional design choices, not bugs; the risk is that integrators performing an `ERC-165` check would assume full compliance. Deviations (MUST violations): 1. Shares are not removed from the owner on request -- Spec: *"the shares MUST be removed from the custody of owner upon requestRedeem."* The implementation only tracks requested shares in accounting (`request.shares += shares`) but leaves them in `balanceOf(owner)`. Because CommitToken is non-transferable, double-use is prevented by the balance check, so this meets the spirit of "removed from custody" but not the letter. 2. `previewRedeem()` and `previewWithdraw()` do not revert -- Spec: *"previewRedeem and previewWithdraw MUST revert for all callers and inputs."* The implementation returns 1:1 conversion. Integrators check these functions to determine if atomic redemption is possible, returning a value implies synchronous redemption is available, which is false. 3. `setOperator()` reverts and operators cannot claim -- Spec: *"MUST set the operator status to the approved value. MUST return True."* The implementation reverts with `NotSupported()`. As a consequence, operators cannot claim on behalf of a controller either (spec: *"the controller MUST be msg.sender unless the controller has approved the msg.sender as an operator"*). The implementation requires `owner == msg.sender` for all claims. Intentional for v0 simplicity. 4. `ERC-7575` `share()` not implemented -- Spec: *"Smart contracts implementing this Vault standard MUST implement the `ERC-7575` standard (in particular the share method)."* Not present. Additional note: The spec uses SHOULD (not MUST) for renaming the `owner` parameter to `controller` on the `redeem()` and `withdraw()` methods. The implementation uses `owner` in `pendingRedeemRequest()`/`claimableRedeemRequest()`. This is a recommendation-level divergence, not a spec violation.

The `CommitToken._withdraw()` function calls `super._withdraw()` (OpenZeppelin `ERC4626._withdraw()` emits a `Withdraw` event), and then emits a `Withdraw` event again. ``` function _withdraw(Request storage request, address caller, address receiver, address owner) internal { ... // audit note: Emitting the first Withdraw event inside the call super._withdraw(caller, receiver, owner, assets, shares); ... // audit note: Emitting the second Withdraw event emit Withdraw(caller, receiver, owner, assets, shares); } ```

`CommitToken` enforces a supply cap in `_update()` and a deny list in `_deposit()`, and inherits `whenNotPaused` on `_deposit()`. However, `maxDeposit()` and `maxMint()` inherit the `ERC4626` defaults that return `type(uint256).max`. Per `ERC-4626`, `maxDeposit` "MUST return the maximum amount of assets deposit would allow to be deposited." and "MUST NOT revert." Currently, calling `deposit(maxDeposit(receiver), receiver)` reverts when `totalSupply < supplyCap` (but non-zero), when the receiver is on the deny list, or when the contract is paused. Integrators that rely on `maxDeposit()` and `maxMint()` to size their calls will experience unexpected reverts.

`mintStatus()` uses `getSchedule(operationId)` to decide if an operation is executable. AccessManager's `getSchedule()` returns `0` when the operation is not scheduled, executed, cancelled, or expired (past `timepoint + expiration()`, default 1 week). The code only treats `scheduleTime > currentTime` or `currentTime < order.notBefore` as "not ready" and otherwise returns `Ready`. It never treats `scheduleTime == 0` as expired. So when the AccessManager schedule has expired but the order's `notAfter` has not, `mintStatus()` still returns `Ready` even though `executeMint()` would revert with `AccessManagerExpired`. This only occurs when the order's validity window is longer than the AccessManager execution window (e.g. `notAfter` set to a long expiry or `type(uint48).max`). For typical short-lived orders (e.g. notAfter within a day or a week), order expiry is hit first and the bug does not appear.