From ac213377a009910c5a85180b5fcec16b434ba86b Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 8 May 2026 16:50:41 +0200 Subject: [PATCH 1/3] docs(contracts): address security audit findings I-03, I-04, I-05 - Fix stale SigQuorum comment on UserState field in AppSessionsV1SubmitDepositStateRequest (I-05) - Add Trust Assumptions section to SECURITY.md documenting Node as off-chain transfer intermediary, Node off-chain credit accounting, and signature validator selection trust; consolidate scattered trust assumptions from Invariants and Signature Validation sections (I-03) - Document on-chain vs off-chain EIP-191/raw-ECDSA signature domain asymmetry in SECURITY.md and protocol-description.md (I-04) --- contracts/SECURITY.md | 58 ++++++++++++++++++++++++++++------------- pkg/rpc/api.go | 2 +- protocol-description.md | 3 +++ 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/contracts/SECURITY.md b/contracts/SECURITY.md index f87347f64..a838e89b5 100644 --- a/contracts/SECURITY.md +++ b/contracts/SECURITY.md @@ -41,17 +41,6 @@ Invariant: --- -- (NOT TRUE) only less-or-equal amount of internally-accounted funds can be withdrawn (NOT TRUE for states that include "receive" off-chain ops) - -The absence of the aforementioned invariant creates a huge risk of an attacker draining the Node. -To protect from this, the Node should keep CORRECT track of off-chain user funds. -CAUTION IS REQUIRED. - -P.S. This invariant still can be enforced by updating `lockedFunds` per channel meta-variable during on-chain state processing, -e.g. when processing "receive X, withdraw Y", increase `lockedFunds` (and "lock" Node's funds in channel) by X, and then decrease by Y. - ---- - - User funds can be withdrawn only after channel is finalized (closed or challenged) or during WITHDRAW action - any action is valid only with a Node's signature (for now, but this condition may be loosened to improve UX by making protocol more complex) - a state with `version` <= `latestKnownVersion` per chain cannot be accepted as valid @@ -182,6 +171,39 @@ no funds can be permanently locked if it does. --- +## Trust Assumptions + +Beyond the cryptographic and on-chain guarantees listed above, correct user fund protection depends on the following trust assumptions about Node behavior. + +### Node as off-chain transfer intermediary + +When a user sends funds off-chain to another party, the protocol requires two independent state updates that the Node must countersign: + +1. The sender's channel state: user allocation decreases, encoding the transfer. +2. The receiver's channel state: receiver allocation increases. + +A malicious Node could countersign the sender's state (making the reduction in user allocation on-chain enforceable) while withholding the receiver's credit state — pocketing the transferred funds. The on-chain contract has no visibility into independent per-channel state updates and cannot enforce atomicity between them. + +**Users must trust the Node to faithfully execute both legs of every off-chain transfer.** This extends the existing Node trust relationship (liquidity management, cross-chain relay) to include off-chain transfer routing. If the Node behaves maliciously, the user's recourse is to challenge the channel and close it, recovering whatever funds remain in their on-chain allocation at the last enforced state. + +### Node off-chain credit accounting + +The protocol does not bound on-chain withdrawals to the amount explicitly deposited. States that include "receive" off-chain operations can increase a user's allocation beyond what was deposited, reflecting credit extended by the Node. A Node that over-issues credits may have insufficient vault funds to honour those allocations when they are enforced on-chain. + +**Users must trust the Node to extend only credit that is backed by actual vault funds.** The Node is solely responsible for maintaining correct off-chain accounting of all user balances and ensuring no signed state represents an allocation the Node cannot fund. + +### Signature validator selection + +In the single-node deployment model, the ChannelHub is deployed by the Node operator, who also chooses the `defaultSigValidator`. The following trust properties apply: + +- **Default validator trust**: All participants using the default validator (0x00) trust the ChannelHub deployer's choice of default validator. +- **User validator control**: Users control which additional validators (beyond the always-available default) can verify their signatures via the `approvedSignatureValidators` bitmask in `ChannelDefinition`. This prevents nodes from forging user signatures by registering malicious validators. +- **Validator agreement**: Both users and nodes can only use agreed validators specified in the bitmask (plus the always-available default validator), preventing unilateral changes to signature validation schemes. +- **Registration immutability**: Once a node registers a validator at a specific ID, it cannot be changed. Signatures created with a given validator ID remain valid for the lifetime of the ChannelHub deployment. +- **Cross-chain consistency**: The same validator ID may map to different validator addresses on different chains, but the security properties must remain equivalent. Nodes are responsible for registering compatible validators across chains. + +--- + ## Signature Validation Security The Nitrolite protocol uses a pluggable signature validation system to support flexible authorization schemes. This section describes the security model and considerations for signature validators. @@ -243,15 +265,15 @@ The protocol maintains clear separation between protocol concerns (ChannelHub) a See `signature-validators.md` for detailed documentation on each validator. -### Trust Model +### On-Chain vs Off-Chain Signature Domain Asymmetry -- **Default validator trust**: All participants using the default validator (0x00) trust the ChannelHub deployer's choice of default validator. -- **User validator control**: Users control which additional validators (beyond the always-available default) can verify signatures via the `approvedSignatureValidators` bitmask in `ChannelDefinition`. This prevents nodes from forging user signatures by registering malicious validators. Users can approve specific validators from the node's registry by setting the corresponding bits. -- **Validator agreement**: Both users and nodes can only use agreed validators specified in the bitmask (plus the always-available default validator). This ensures that validators are mutually agreed upon and prevents unilateral changes to signature validation schemes. -- **Registration immutability**: Once a node registers a validator at a specific ID, it cannot be changed. This ensures that signatures created with a given validator ID remain valid for the lifetime of the ChannelHub deployment. -- **Cross-chain consistency**: The same validator ID may map to different validator addresses on different chains, but the security properties must remain equivalent. Nodes are responsible for registering compatible validators across chains. +The default ECDSA validator (`EcdsaSignatureUtils.validateEcdsaSigner`) accepts **both EIP-191 and raw ECDSA** signatures on-chain: it first attempts EIP-191 recovery (the `"\x19Ethereum Signed Message:\n"` prefix used by `eth_sign`), then falls back to raw `keccak256(message)` recovery if EIP-191 fails. ---- +The Nitronode off-chain validator (`ChannelSigValidator`) uses **EIP-191 only** — no raw-ECDSA fallback. + +**Consequence:** A channel state signature produced with raw `keccak256` (no EIP-191 prefix) will pass on-chain validation but be rejected off-chain by the Nitronode. Under correct protocol operation this cannot occur, because the Nitronode only countersigns states it has already verified off-chain. However, implementations building custom signers or relayers must be aware of this asymmetry. + +**All client implementations must use EIP-191 (`eth_sign`) for channel state signatures** to ensure round-trip validity on both the off-chain (Nitronode) and on-chain (ChannelHub) validation paths. The raw-ECDSA fallback exists in the on-chain contract as a compatibility measure and should not be relied upon for new integrations. ### Bootstrap vulnerability: initial user signature at `createChannel` diff --git a/pkg/rpc/api.go b/pkg/rpc/api.go index a67ae12e3..bd6c7a856 100644 --- a/pkg/rpc/api.go +++ b/pkg/rpc/api.go @@ -148,7 +148,7 @@ type AppSessionsV1SubmitDepositStateRequest struct { AppStateUpdate AppStateUpdateV1 `json:"app_state_update"` // QuorumSigs is the list of participant signatures for the app state update QuorumSigs []string `json:"quorum_sigs"` - // SigQuorum is the signature quorum for the application session + // UserState is the signed channel state from the user, used to fund the application session deposit UserState StateV1 `json:"user_state"` } diff --git a/protocol-description.md b/protocol-description.md index 9cab99a04..b633926f3 100644 --- a/protocol-description.md +++ b/protocol-description.md @@ -595,6 +595,8 @@ This works because `prevStoredState` was swapped during `INITIATE_MIGRATION`. 2. **Node fund lock**: User forces Node to lock large funds via escrow deposit, then blocks all recovery operations with minimal capital. * Combined gas limiting + reclaim pattern ensures channel operations continue regardless of transfer success. +* **Node trust for off-chain transfer routing**: Off-chain transfers between parties are routed through the Node. The sender signs a state where their allocation decreases; the Node is expected to countersign it and also countersign a corresponding credit state for the receiver. The on-chain contract cannot enforce atomicity between two independent channel updates. A malicious Node could apply the sender's state while withholding the receiver's credit, effectively capturing the transferred funds. Users must trust the Node to faithfully execute both legs of every off-chain transfer. + --- ## Signature validation @@ -614,6 +616,7 @@ Since `approvedSignatureValidators` is part of the `channelId` computation, agre * Zero transaction overhead (no separate validator registration needed) * Prevents node-controlled validator forgery attacks * Default ECDSA validator always available as fallback +* **Signature domain**: The default on-chain ECDSA validator accepts both EIP-191 (`eth_sign`) and raw `keccak256` signatures (this is done for extensibility), while the Nitronode off-chain validator accepts EIP-191 only. All client-produced signatures must use EIP-191 to be valid on both paths. ### Node validator registry From 1cb79b70b42daf9e4a618af364110c399e841817 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Fri, 8 May 2026 16:55:33 +0200 Subject: [PATCH 2/3] docs(contracts): document IERC20Metadata.decimals() requirement for supported tokens --- contracts/SECURITY.md | 6 ++++++ contracts/src/Utils.sol | 5 ++++- protocol-description.md | 3 ++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/contracts/SECURITY.md b/contracts/SECURITY.md index a838e89b5..998d21378 100644 --- a/contracts/SECURITY.md +++ b/contracts/SECURITY.md @@ -405,6 +405,12 @@ Use non-rebasing equivalents where available (e.g. wstETH instead of stETH). Fee-on-transfer tokens are **not supported**. The amount received by the contract is less than the amount recorded in the ledger, causing it to overstate holdings from the very first deposit. This produces the same class of insolvency as a negative rebase: late withdrawers may receive less than recorded or nothing at all. +### Tokens without `decimals()` + +ERC-20 defines `decimals()` as an **optional** extension (via `IERC20Metadata`), not a core requirement. However, during token decimals validation a `IERC20Metadata(token).decimals()` is called, which will revert the outer transaction if the call fails. This check runs on every channel state transition, escrow deposit, and escrow withdrawal — meaning tokens that omit `decimals()` **cannot be used at all**, even for otherwise supported operations. + +Tokens must implement `IERC20Metadata.decimals()`. Unlike the rebasing and fee-on-transfer restrictions (which are accounting-correctness requirements), this is a hard on-chain gate: the contract will reject the transaction immediately rather than silently misaccount. + --- ## Native ETH vs ERC20 Deposit Asymmetry diff --git a/contracts/src/Utils.sol b/contracts/src/Utils.sol index 9a5ab1183..dbfd4cfc7 100644 --- a/contracts/src/Utils.sol +++ b/contracts/src/Utils.sol @@ -80,7 +80,10 @@ library Utils { /** * @notice Validates that the ledger's decimals match the token contract's decimals - * @dev Only validates if on the same chain as the ledger + * @dev Only validates if on the same chain as the ledger. + * The token must implement IERC20Metadata.decimals(). If the call reverts, + * this function reverts with FailedToFetchDecimals. ERC-20 tokens that omit + * the optional decimals() method cannot be used in any protocol operation. * @param ledger The ledger to validate */ function validateTokenDecimals(Ledger memory ledger) internal view { diff --git a/protocol-description.md b/protocol-description.md index b633926f3..1cb97f2ae 100644 --- a/protocol-description.md +++ b/protocol-description.md @@ -582,8 +582,9 @@ This works because `prevStoredState` was swapped during `INITIATE_MIGRATION`. * **Rebasing tokens** (e.g. stETH, aTokens): their autonomous balance changes are invisible to the ledger and create unrecoverable accounting divergence. Use non-rebasing wrappers instead (e.g. wstETH). * **Fee-on-transfer tokens**: the amount received by the contract is less than the amount recorded, causing the ledger to overstate holdings from the very first deposit. + * **Tokens without `decimals()`**: the contract calls `IERC20Metadata.decimals()` on every state transition; tokens that do not implement this optional ERC-20 extension are rejected on-chain with `FailedToFetchDecimals`. Unlike rebasing and fee-on-transfer issues (which silently corrupt accounting), missing `decimals()` causes an immediate hard revert. - There is no hard-coded guardrail preventing deposit of these tokens — the contract will accept them, but any discrepancy will produce undefined accounting behavior for all users of that token. Enforcement is off-chain: the Node will not sign states that reference unsupported token types. + There is no hard-coded guardrail preventing deposit of rebasing or fee-on-transfer tokens — the contract will accept them, but any discrepancy will produce undefined accounting behavior for all users of that token. Enforcement is off-chain: the Node will not sign states that reference unsupported token types. * **Transfer failure resilience**: Outbound transfers (to users) never revert on failure: From d7825dcf93b52df4e014abca31af45b539ca1d54 Mon Sep 17 00:00:00 2001 From: nksazonov Date: Mon, 11 May 2026 09:58:29 +0200 Subject: [PATCH 3/3] docs(protocol): mirror I-03 off-chain transfer routing trust assumption to security-and-limitations.md --- docs/protocol/security-and-limitations.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/protocol/security-and-limitations.md b/docs/protocol/security-and-limitations.md index c7dae6e2e..a54575f0b 100644 --- a/docs/protocol/security-and-limitations.md +++ b/docs/protocol/security-and-limitations.md @@ -62,6 +62,7 @@ In the current protocol version, participants MUST trust nodes for: - **Cross-chain liquidity** — nodes MUST maintain sufficient funds on each supported chain to honour off-chain allocations; insufficient liquidity may cause on-chain enforcement to fail - **Cross-chain relay** — nodes relay cross-chain state updates; trustless cross-chain enforcement is not yet implemented - **Timely enforcement** — nodes are expected to submit checkpoints when requested; delayed enforcement may affect user experience but does not compromise single-chain asset safety +- **Off-chain transfer routing** — when a user sends funds off-chain to another party, the node must countersign both the sender's state (decreasing their allocation) and the receiver's credit state (increasing theirs); the on-chain contract cannot enforce atomicity between two independent channel updates. A malicious node could apply the sender's state while withholding the receiver's credit, capturing the transferred funds. Users must trust the node to faithfully execute both legs of every off-chain transfer. Participants do not need to trust nodes for: