Add receiver-side resource limits for untrusted peers (#184)#185
Add receiver-side resource limits for untrusted peers (#184)#185ndisidore wants to merge 1 commit into
Conversation
Deserializing messages from an untrusted peer could exhaust CPU, memory, or the stack: - A long [bigint,...] digit string fed straight to BigInt() blocks the event loop, because BigInt()'s decimal parse is superlinear. - A deeply nested message could overflow the stack during evaluation. - An arbitrarily large message was JSON-parsed with no up-front size bound. Add three local, receiver-side guards in the deserialization path: - Cap bigint digit length and reject non-numeric strings before BigInt(). - Bound nesting depth in Evaluator.evaluateImpl, mirroring the existing send-side depth limit in Devaluator. - Reject oversized messages in the session read loop before JSON.parse. Limits have safe exported defaults (DEFAULT_LIMITS) and are overridable per session via a new RpcLimits-typed 'limits' field on RpcSessionOptions, surfaced to the Evaluator through the Importer interface so the standalone deserialize() path stays protected by the defaults. Limits are purely local (the protocol has no negotiation step); exceeding one throws, which aborts the session via the existing abort path.
🦋 Changeset detectedLatest commit: a6ac783 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
All contributors have signed the CLA ✍️ ✅ |
commit: |
|
I have read the CLA Document and I hereby sign the CLA |
| throw new TypeError( | ||
| `Deserialized bigint exceeds maximum length of ${maxBigIntDigits} digits.`); | ||
| } | ||
| // Reject anything BigInt() would parse expensively or unexpectedly before handing it |
There was a problem hiding this comment.
To be discussed:
I think this was a bug/unintentional behavior before.
protocol.md:181: "A bigint value, represented as a decimal string."
It is technically possible now to handcraft ["bigint","0xff"] which unconditionally calls BigInt(value[1]) meaning this gets parsed as 255n.
But this means there is a disconnect in the protocol writeup and the actual implementation
1 participant
Summary
Deserializing untrusted input could exhaust a peer's resources. Most concretely (#184), a
["bigint", "<digits>"]wire value was handed straight toBigInt()with no length check, andBigInt()decimal parsing is superlinear — so a multi-megabyte digit string blocks the event loop. While fixing that, the receive path is also bounded against the related single-message exhaustion vectors the issue calls out ("review that we're properly limiting message sizes overall").The guards are local, receiver-side decisions that fit capnweb's existing design: no new dependencies, no protocol changes, and reuse of the existing abort path on violation.
What this changes
BigInt().evaluateImplnow tracks recursion depth and rejects messages nested beyond the limit (mirroring the existing send-side depth cap), so a tiny deeply-nested message can't overflow the stack.JSON.parse, bounding the up-front allocation.Configuration
Limits are always applied from the exported
DEFAULT_LIMITS, and can be overridden per session via a new optionallimitsfield onRpcSessionOptions:Defaults:
maxBigIntDigits: 16384— 2^14 digits ≈ ~54,000 bits, far beyond any practical cryptographic integer (an RSA-16384 modulus is ~4,933 decimal digits), so no legitimate value is rejected — yet orders of magnitude below the millions of digits needed forBigInt()'s superlinear parse to block meaningfully.maxDepth: 64— matches the existing send-side depth limit, so capnweb never rejects a message it would itself send.maxMessageSize: 32 MiB— generous for legitimate batched and base64/blob payloads while still bounding a single allocation.Because the protocol has no negotiation step and a limit is a purely local receiver-side decision, the defaults are deliberately generous: a sender cannot discover the receiver's limit, so the defaults must never trip legitimate traffic. They are tunable per session for endpoints with unusual needs.
Behavior on violation
Exceeding any limit throws, which flows through the existing readLoop →
abort()path: the session sends an["abort", ...]frame and tears down. The thrown errors name the limit and its value, so a cooperative peer sees a descriptive reason on the aborted session — riding the abort frame that is already sent, with no protocol change.Alternatives considered
deserialize()path, but leaves no escape hatch for applications that legitimately exchange very large payloads. Kept the hardcoded defaults as the floor, but made them overridable.getLimits()RPC method — purely a cooperative-peer ergonomic that needs no library change (an application can already expose such a method); it is not a substitute for local enforcement, so it is left to applications that want it.Deferred (potential follow-ups)
These are larger resource-exhaustion concerns from the issue discussion that need rate/quota or backpressure design rather than a per-message guard, and are intentionally out of scope here:
push/stream/pipemessages (the "many messages → OOM" case) — needs per-session quotas and backpressure.remapinstruction/capture fan-out producing multiplicative work.Note: nested call-arguments are deserialized as a fresh payload with a fresh depth budget, so a message that nests deeply through call arguments can still recurse beyond
maxDepth. This fails safe — it is bounded bymaxMessageSizeand produces a clean session abort (a caughtRangeError), not a process crash (verified) — and belongs to the same follow-up bucket.Testing
__tests__/limits.test.ts(14 tests): just-under / over boundaries for each limit, non-numeric bigint rejection, per-session override enforcement, the standalonedeserialize()protected by defaults, and backwards-compatibility — sessions constructed with no options, and with empty options, round-trip normal bigints / nesting / calls unchanged.