Skip to content

Security Audit Remediation — 2026-03-20

Audit date: 2026-03-20 Remediation date: 2026-03-20 Scope: Full-stack audit (smart contracts + backend + frontend + infrastructure)

Remediation Summary

Severity Total Code-fixed Operationally mitigated Deferred
CRITICAL 4 4 0 0
HIGH 9 8 1 0
MEDIUM 14 12 1 1
LOW 10 9 1 0
INFO 8 — — —
TOTAL 45 33 3 1

6 contract-level fixes (H-05, H-06, L-04, L-05, L-06, L-07) are code-fixed in source and take effect upon contract redeployment. INFO findings are confirmed non-issues requiring no remediation. The sole deferred finding is M-14 (npm tmp dependency in solc).

CRITICAL Findings

C-01: POST /v1/seats/generate Has No Authentication

  • Status: FIXED
  • Commit: 21f2f4b (fix(security): remediate critical auth and key-material audit findings)
  • Fix: Added /v1/seats/generate to isAdminRoute() POST section and requiredRole() returning 'ADMIN'.
  • Test: test/security.test.ts — 3 tests (unauthenticated 401, invalid token 401, VIEWER 403). OPERATOR 403 is covered under H-02.
  • Remaining risk: None. Endpoint now requires ADMIN-level authentication.

C-02: BLS Mnemonic and Keystore Password Exposed in /proc/pid/cmdline

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Both runDepositCli() (generate.ts) and runDepositCliBatch() (data.ts) now pass secrets via spawnSync({ input: stdinData }) instead of --mnemonic and --keystore_password CLI arguments. deposit-cli prompts for these values when not provided as flags, even in --non_interactive mode, and reads them from stdin.
  • Test: test/security.test.ts — 3 tests verify source code no longer contains --mnemonic or --keystore_password in args arrays, and that input: stdinData is used.
  • Remaining risk: The mnemonic is still present in the Node.js process's environment variables (/proc/<pid>/environ), which is readable by the same UID. This is a smaller exposure than world-readable /proc/<pid>/cmdline. Full elimination would require a dedicated secrets manager or HSM integration, which is out of scope for this pass.
  • Verification: Confirmed via manual testing that deposit-cli reads mnemonic and keystore password from stdin when invoked without --mnemonic and --keystore_password flags.

C-03: Default Keystore Password + PBKDF2 = Zero-Cost Key Extraction

  • Status: PARTIALLY FIXED
  • Commit: 21f2f4b
  • Fix applied:
  • Removed hardcoded default password 'seat-manager-live-intents-123' from both getKeystorePassword() (generate.ts) and runDepositCliBatch() (data.ts).
  • Both now require SEAT_MANAGER_INTENTS_KEYSTORE_PASSWORD env var, failing with a clear error if unset.
  • validateKeygenPrerequisites() also checks the env var is set.
  • Test: test/security.test.ts — validates validateKeygenPrerequisites reports missing password.
  • Remaining risk / migration gap:
  • Existing keystores in the SeatBlsMaterial table were encrypted with the old default password and PBKDF2. They remain trivially decryptable by anyone with DB read access and knowledge of the old default password (which is in git history).
  • PBKDF2 vs scrypt: deposit-cli is still invoked with --pbkdf2 for speed. Removing this flag would use scrypt but significantly slow key generation. This is a deployment-time trade-off.
  • Per-seat random passwords: The audit recommended per-seat random passwords stored in a separate secrets manager. This requires a full key management redesign and is deferred.
  • Legacy material disposition: Existing keystores created with the old default password on this disposable mainnet are invalid test material. They must not be promoted to any serious future environment. No rotation project is warranted — new validators will use properly configured passwords.
  • Future consideration: For a production environment, evaluate removing the --pbkdf2 flag and integrating with a secrets manager (Vault, AWS SSM) for per-seat keystore passwords.

C-04: Default Test Mnemonic Auto-Created in Production Path

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Removed DEFAULT_TEST_MNEMONIC constant and the auto-creation branch in loadOrInitMnemonic(). The function now throws 'BLS mnemonic not configured' if neither the env var nor the mnemonic file exists.
  • Test: test/security.test.ts — validates loadOrInitMnemonic() throws when no mnemonic is configured.
  • Remaining risk: None. There is no longer a code path that creates or uses a test mnemonic.

HIGH Findings

H-01: Non-Constant-Time Admin Token Comparison

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Replaced token !== options.adminToken with timingSafeEqual(Buffer.from(token), Buffer.from(expectedBuf)), including a length pre-check.
  • Test: test/security.test.ts — validates wrong token is rejected (functional correctness); timing safety is structural.
  • Remaining risk: None.

H-02: OPERATOR Role Can Execute Irreversible Contract Governance Operations

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: requiredRole() now returns 'ADMIN' for /v1/seat-manager/freeze/*, /v1/seat-manager/switch, and /v1/seats/generate.
  • Test: test/security.test.ts — 5 tests verify OPERATOR gets 403 on all governance routes, and 1 test verifies ADMIN passes auth on freeze/schedule.
  • Remaining risk: None.

H-03: authenticateUser Leaks Username Existence via Timing Side-Channel

  • Status: FIXED
  • Commit: 70339b2 (fix(auth): harden session validation and login timing behavior)
  • Fix: When user is not found or disabled, authenticateUser() now calls scryptSync(password, 'timing-cover-salt', SCRYPT_KEYLEN) before returning undefined, matching the timing of a real password verification.
  • Test: test/security.test.ts — validates source contains timing cover, and functional tests for all three paths (missing user, wrong password, correct credentials).
  • Remaining risk: The cover uses a fixed salt, so the timing may not be exactly identical to a real verify (which uses a per-user salt). The difference is sub-millisecond and not exploitable in practice over a network.

H-04: scryptSync Blocks Event Loop on Every Authenticated Request

  • Status: FIXED
  • Commit: 70339b2
  • Fix: Replaced scryptSync(token, 'session-salt', SCRYPT_KEYLEN) in both createSession() and validateSession() with createHmac('sha256', SESSION_HMAC_KEY).update(token).digest('hex'). Session tokens have 256-bit CSPRNG entropy, making HMAC-SHA256 cryptographically sufficient.
  • Test: test/security.test.ts — validates round-trip (create → validate), rejection of invalid tokens, and source code uses hashSessionToken not scryptSync.
  • Remaining risk: SESSION_HMAC_SECRET env var should be set in production for session persistence across process restarts. Without it, a random key is generated per process, invalidating all sessions on restart.
  • Migration note: All existing sessions are invalidated by the hash algorithm change. Users must re-login after deployment.

H-05: Re-Allowlisting Consumed Deposit Intents (No Consumed Flag)

  • Status: FIXED — contract updated, requires redeployment
  • Fix: Added mapping(bytes32 => bool) public consumedIntents to DepositContractCTN.sol. The deposit() function now sets consumedIntents[intentHash] = true on consumption. Both addAllowedDeposit() and addAllowedDeposits() revert with DepositIntentAlreadyConsumed if the intent was previously consumed.
  • Test: test_TopUp_RevertsAfterIntentConsumed validates that re-allowlisting a consumed intent reverts.
  • Remaining risk: None once the contract is redeployed.

H-06: Beneficiary/Treasury as Reverting Contract = Permanent Fund Lock

  • Status: MITIGATED — deployment-time policy, not a runtime guarantee
  • Fix: Added extcodesize checks in the WithdrawalVault constructor. If either treasury or beneficiary has code at deployment time, the constructor reverts with RecipientIsContract().
  • Test: test_Constructor_ContractTreasury_Reverts and test_Constructor_ContractBeneficiary_Reverts validate the guard.
  • Honest assessment: This is a deployment-time check only. It prevents accidental misconfiguration (e.g. passing a multisig address) but does not provide a runtime guarantee. Three bypass scenarios exist:
  • CREATE2 pre-image: An address that is an EOA at deploy time but later becomes a contract via CREATE2.
  • EIP-7702 delegation: A future protocol upgrade (or existing on chains that support it) could let an EOA delegate to code that reverts on receive().
  • Self-destructing proxy: A contract that self-destructs before vault deployment, then is re-deployed at the same address.
  • Operational mitigation: The seat manager only deploys vaults with operator-provided or treasury-controlled addresses. Attackers cannot inject arbitrary addresses unless they compromise the admin key. This is a design tradeoff: deployment-time policy + operational controls, not a cryptographic guarantee.

H-07: Floating-Point Precision Loss in principalTargetWei Calculation

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Replaced BigInt(Math.round(parseFloat(principalCtn) * 1e18)) with parseEther(principalCtn) from viem, which handles arbitrary-precision decimal-to-wei conversion.
  • Test: test/security.test.ts — validates parseEther produces exact values for edge cases like '0.1' and '1.123456789012345678'.
  • Remaining risk: None.

H-08: Orphaned Vault on Crash in create-with-vault Flow

  • Status: FIXED
  • Fix: Rewrote the API POST /v1/seats/with-vault handler with a full SeatOperation journal pattern. The flow now:
  • Creates a SeatOperation record (status STARTED) with an idempotency check before any on-chain work
  • Deploys the vault, then advances the operation to VAULT_DEPLOYED (recording vault address + tx hash)
  • Creates the DB seat record, then advances to COMPLETED
  • On handler entry, checks for any incomplete operations and resumes from the last checkpoint
  • File: src/api/fastifyServer.ts — POST /v1/seats/with-vault handler
  • Remaining risk: The CLI seatCreateWithVaultCommand does not yet use the journal pattern. CLI usage is operator-only and manual recovery is straightforward.

H-09: Static Admin Token Bypasses All RBAC + Cannot Download Keystores

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: The static token auth branch now sets request.authenticatedUser to a synthetic user with role: 'ADMIN', username: 'static-admin'. This ensures:
  • RBAC enforcement applies (the ADMIN role passes all checks)
  • Audit logs record 'static-admin' instead of generic 'admin'
  • Keystore download works (the !request.authenticatedUser guard passes)
  • Test: test/security.test.ts — validates static token authenticates, accesses ADMIN routes, and wrong token is rejected.
  • Remaining risk: The static token mechanism is still a single shared secret with no rotation or revocation. Session-based auth should be preferred for production use.

MEDIUM Findings

M-01: GET /v1/operators/:id Has No Authentication

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Added if (/^\/v1\/operators\/[^/]+$/.test(path)) return true; to the GET section of isAdminRoute().
  • Test: test/security.test.ts — validates unauthenticated request returns 401 and authenticated request passes.

M-07: EL Watcher Silently Swallows All Errors in Event Processing

  • Status: FIXED
  • Commit: 5bdd6b8
  • Fix: Restructured inner loop so try/catch only wraps decodeEventLog() (with continue on failure). DB operations (recordDeposit, updateSeatStatus) are now outside the catch and propagate to the outer error handler, which logs the error and triggers a retry on the next poll cycle.
  • Test: test/security.test.ts — structural verification that the decode catch only contains continue and DB calls are outside it.

M-10: Keystore Checksum Function Mismatch

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: generateValidatorKey() now uses JSON.stringify(keystoreJson) for the checksum, consistent with computeKeystoreChecksum().
  • Remaining risk: Existing checksums in the database were computed from raw file strings and may not match computeKeystoreChecksum() output. This only affects integrity verification of pre-existing keystores.

M-11: Missing spawnSync Timeout in Batch Deposit Data Generation

  • Status: FIXED
  • Commit: 21f2f4b
  • Fix: Added timeout: 60_000 to the spawnSync call in runDepositCliBatch() (data.ts).

M-12: Error Handler Leaks Internal Error Messages to Client

  • Status: FIXED
  • Commit: 5bdd6b8
  • Fix: Error handler now returns 'Internal server error' for 5xx responses. 4xx errors still return the original message (needed for validation feedback). All error responses include requestId for correlation with server-side logs.
  • Test: test/security.test.ts — validates 5xx response contains generic message and requestId.

M-04: No Operator Isolation in Seat Operations

  • Status: FIXED (escalated to ADMIN)
  • Commit: 5bdd6b8
  • Fix: requiredRole() now returns 'ADMIN' for /v1/seats/:id/(approve|revoke|deposit). There is no User→Operator mapping in the schema, so OPERATOR role cannot be safely scoped to own seats. Promoting to ADMIN-only is the safe default. OPERATOR can still create seats and read data.
  • Test: test/security.test.ts — 4 tests (OPERATOR 403 on approve/revoke/deposit, ADMIN pass-through on approve).
  • Future: If per-operator isolation is needed, add an operatorId FK to the User model and check ownership in the route handlers.

M-05: No CSP or Security Headers on Admin Frontend

  • Status: FIXED
  • Commit: 5bdd6b8
  • Fix: Added headers() to apps/admin/next.config.ts returning X-Frame-Options DENY, X-Content-Type-Options nosniff, Referrer-Policy strict-origin-when-cross-origin, X-DNS-Prefetch-Control off, and a CSP with frame-ancestors 'none'. CSP allows 'unsafe-inline' and 'unsafe-eval' for Next.js hydration/dev compatibility.
  • Remaining risk: 'unsafe-inline' and 'unsafe-eval' weaken CSP protection against XSS. A nonce-based CSP would be stronger but requires Next.js middleware configuration. Combined with session tokens in localStorage (known issue #3), XSS remains the primary frontend risk.

M-02: Mnemonic Index Allocation Race in Generate Flow

  • Status: FIXED
  • Fix: Removed the separate allocateMnemonicIndex() call from the API handler. Index allocation now happens atomically inside findOrCreateSeatOperation(), after the idempotency check, ensuring two concurrent requests cannot claim the same index.
  • File: src/api/fastifyServer.ts (generate handler), src/db/prismaStore.ts (findOrCreateSeatOperation)

M-03: Idempotency Key TOCTOU Race

  • Status: FIXED
  • Fix: The idempotency middleware now stores the key in a pending state (HTTP 202) immediately before processing the request. A concurrent request with the same key receives a 409 Conflict. Upon completion, the stored entry is updated with the actual response status and body.
  • File: src/api/fastifyServer.ts — idempotency middleware

M-06: EL Watcher Processes Deposits from Non-Finalized Blocks (Reorg Risk)

  • Status: FIXED
  • Fix: EL watcher now queries client.getBlock({ blockTag: 'finalized' }) instead of client.getBlockNumber(). Falls back to getBlockNumber() if the chain doesn't support the finalized tag.
  • File: src/watcher/el-watcher.ts

M-08: CL Watcher Does Not Detect Slashed Validators

  • Status: FIXED
  • Fix: checkValidatorVisible() now inspects the validator's status field from the beacon API response. Validators with status active_slashed or exited_slashed are treated as not visible, preventing them from advancing to ACTIVE.
  • File: src/watcher/cl-watcher.ts

M-09: CL Quorum Is Single-Source (Same Client, Same Peer Path)

  • Status: MITIGATED — design limitation
  • Rationale: Both CL endpoints use the same Lighthouse client and peer through the same path. This is a known infrastructure constraint of the two-node mainnet topology, not a software bug. Adding a third-party CL node or a different client (e.g., Teku) would provide true diversity. Documented as a known infrastructure limitation.

M-13: CORS Wildcard Origin in Production

  • Status: FIXED
  • Fix: Added a runtime warning when allowedOrigins contains '*', logged at startup via log.warn('CORS wildcard origin (*) is configured — do not use in production'). The CORS configuration now uses an IIFE to check for wildcards and emit the warning.
  • File: src/api/fastifyServer.ts — CORS plugin registration

M-14: solc npm Package Pulls Vulnerable tmp Dependency

  • Status: DEFERRED
  • Rationale: The solc npm package is used at build time for vault contract compilation. It transitively depends on tmp@0.0.33, which has a known advisory. The exposure is limited to the build environment (not runtime). Mitigation options: (a) pre-compile the vault ABI/bytecode artifact and remove the solc dependency, or (b) wait for an upstream solc release that updates tmp. Neither is urgent for the current deployment.

LOW Findings

L-01: Static Session Salt in scryptSync

  • Status: MITIGATED — superseded by H-04
  • Rationale: Session hashing was replaced with HMAC-SHA256 using a per-process secret key (H-04 fix). The static salt is no longer used in any code path.

L-02: API Accepts Arbitrary Deposit Amounts

  • Status: FIXED
  • Fix: Added .refine((v) => v === '32', { message: 'Deposit amount must be exactly 32 CTN' }) to depositSeatSchema.amountCtn. The API now rejects any deposit amount other than exactly '32'.
  • File: src/api/schemas.ts

L-03: Login Endpoint Has No Rate Limiting

  • Status: FIXED
  • Fix: Added an in-memory rate limiter to POST /v1/auth/login. Limits to 5 attempts per IP address per 15-minute window. Returns 429 with Retry-After header when exceeded. Rate state is cleaned up on each request check.
  • File: src/api/fastifyServer.ts — login route handler

L-04: cancelPendingClaim Missing nonReentrant Modifier

  • Status: FIXED — contract updated, requires redeployment
  • Fix: Added nonReentrant modifier to cancelPendingClaim() in WithdrawalVault.sol.

L-05: Treasury == Beneficiary Not Prevented

  • Status: FIXED — contract updated, requires redeployment
  • Fix: Constructor now reverts with TreasuryBeneficiarySame() if both addresses match.
  • Test: test_Constructor_TreasuryEqualsBeneficiary_Reverts

L-06: cancelOwnershipTransfer Emits No Event

  • Status: FIXED — contract updated, requires redeployment
  • Fix: Added OwnershipTransferCancelled(address indexed owner, address indexed cancelledPendingOwner) event to DepositContractCTN.sol, emitted in cancelOwnershipTransfer().
  • Test: test_CancelOwnershipTransfer verifies event emission with correct parameters.

L-07: Unbounded Constructor Parameters

  • Status: FIXED — contract updated, requires redeployment
  • Fix: Added MAX_CLAIM_DELAY = 30 days and MAX_PRINCIPAL_TARGET = 10_000 ether constants with constructor checks that revert with ClaimDelayTooLong or PrincipalTargetTooHigh.
  • Tests: test_Constructor_ClaimDelayTooLong_Reverts, test_Constructor_PrincipalTargetTooHigh_Reverts

L-08: CLI Deposit Command Does Not Check Seat Status

  • Status: FIXED
  • Fix: The CLI deposit command now checks seat.status === 'ALLOWLISTED' before proceeding. If the seat is in any other status, it throws 'Seat <id> is in status <status>, expected ALLOWLISTED for deposit'.
  • File: src/cli/seat.ts

L-09: CLI Deposit Does Not Update Seat Status After Success

  • Status: FIXED
  • Fix: After a successful on-chain deposit transaction, the CLI now calls store.updateSeatStatus(seat.id, 'DEPOSITED') to immediately advance the seat status without waiting for the EL watcher.
  • File: src/cli/seat.ts

L-10: CL Observations Table Grows Without Bound

  • Status: FIXED
  • Fix: Added cleanOldClObservations(retentionMs) to PrismaSeatStore (default 7-day retention). The CL watcher runs cleanup hourly via a periodic check in the main loop.
  • Files: src/db/prismaStore.ts, src/watcher/cl-watcher.ts

INFORMATIONAL Findings (I-01 through I-08)

All informational findings are CONFIRMED and require no remediation:

  • I-01: solc pinned with integrity hash
  • I-02: No dangerouslySetInnerHTML
  • I-03: Prisma prevents SQL injection
  • I-04: SeatOperation journal provides crash recovery
  • I-05: Conservation law verified across 12+ scenarios
  • I-06: Intent hash computation matches TS/Solidity
  • I-07: Vault ABI matches deployed contract
  • I-08: 7-property post-deploy verification

Findings Requiring Contract Redeployment

All 6 findings are now code-fixed in source and take effect upon contract redeployment:

Finding Contract Fix applied
H-05 DepositContractCTN consumedIntents mapping + revert on re-allowlist
H-06 WithdrawalVault extcodesize check on treasury/beneficiary in constructor
L-04 WithdrawalVault nonReentrant added to cancelPendingClaim
L-05 WithdrawalVault TreasuryBeneficiarySame revert in constructor
L-06 DepositContractCTN OwnershipTransferCancelled event in cancelOwnershipTransfer
L-07 WithdrawalVault MAX_CLAIM_DELAY (30 days) + MAX_PRINCIPAL_TARGET (10,000 CTN) bounds

Test coverage: 8 new Foundry tests validate these guards (155 total, all passing).

Design/Deployment Assumptions vs Exploitable Bugs

Several findings described deployment-time misconfiguration risks. All are now enforced on-chain:

  • H-06 (reverting recipients): Deployment-time check — constructor reverts if treasury/beneficiary has code. Not a runtime guarantee (see H-06 section for honest assessment).
  • H-05 (re-allowlisting): Now enforced — consumed intents cannot be re-allowlisted.
  • L-07 (unbounded constructor params): Now enforced — upper bounds on principal target and claim delay.
  • L-05 (treasury == beneficiary): Now enforced — constructor reverts if both addresses match.

Corrected Interpretations

H-01 (timing oracle) — severity is accurate but exploitability is limited

The audit correctly identifies non-constant-time comparison. However, exploiting this over a network requires thousands of measurements per character with sub-microsecond accuracy. Over localhost (the only bind address), this is theoretically possible but practically difficult. Fixed regardless as a best practice.

M-09 (CL quorum) — design limitation, not a vulnerability

The audit notes that both CL endpoints use the same Lighthouse client and peer through the same path. This is a known infrastructure constraint of the two-node mainnet topology, not a software bug. Documented as a known infrastructure limitation.

C-02 (proc exposure) — /proc/<pid>/environ vs /proc/<pid>/cmdline

The audit treats /proc/<pid>/environ (env vars) and /proc/<pid>/cmdline (CLI args) as equivalent risks. On modern Linux, cmdline is world-readable (0444) while environ is owner-readable only (0400). Moving secrets from args to stdin eliminates the world-readable exposure. The remaining environ exposure requires same-UID access.

Test Coverage Added

33 tests in test/security.test.ts:

Finding Tests What's validated
C-01 3 Unauthenticated 401, invalid token 401, VIEWER 403
C-02 3 Source code: no --mnemonic/--keystore_password in args, stdin used
C-03 1 validateKeygenPrerequisites requires password env var
C-04 1 loadOrInitMnemonic throws when unconfigured
H-02 6 OPERATOR 403 on generate/freeze/switch, ADMIN passes
H-03 4 Functional auth tests + source contains timing cover
H-04 3 Session round-trip, invalid rejection, source uses HMAC
H-07 1 parseEther exact conversion
H-09 3 Static token auth, ADMIN access, wrong token rejected
M-01 2 Unauthenticated 401, authenticated passes
M-04 4 OPERATOR 403 on approve/revoke/deposit, ADMIN pass-through
M-07 1 Source: decode catch only contains continue, DB ops outside
M-12 1 5xx returns generic message + requestId

Deployment Checklist

Before deploying this remediation:

  1. Set required env vars:
  2. SEAT_MANAGER_INTENTS_MNEMONIC — BLS master mnemonic
  3. SEAT_MANAGER_INTENTS_KEYSTORE_PASSWORD — keystore encryption password (min 12 chars, not the old default)
  4. SESSION_HMAC_SECRET — HMAC key for session tokens (optional, random per process if unset)
  5. All existing sessions are invalidated — users must re-login after deployment.
  6. Existing keystores encrypted with the old default password on this disposable mainnet are invalid test material and must not be reused in any serious environment.
  7. deposit-cli stdin support was verified on the deployed binary. If the deposit-cli binary is updated, re-verify that it reads mnemonic and keystore password from stdin when --non_interactive is used without --mnemonic/--keystore_password flags.