From da021746ab0df20c9f91d37ab5d878900c3f563d Mon Sep 17 00:00:00 2001 From: Ashray Chowdhry Date: Mon, 18 May 2026 09:01:04 -0400 Subject: [PATCH 1/6] fix(connection): block reserved internal event types from server messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A malicious or compromised rippled server could previously send `{"type":"connected"}`, `{"type":"disconnected"}`, `{"type":"error"}`, or `{"type":"reconnect"}` and have `Connection.onMessage` forward those events to internal Connection/Client listeners — spoofing connection state, faking errors, or triggering reconnect logic. `onMessage` now refuses to forward server-supplied `data.type` values that collide with these reserved internal event names. Such messages are dropped and surfaced as a `badMessage` error. Unknown server event types (e.g. future rippled stream types) continue to be forwarded so forward-compatibility is preserved. Fixes XRPLF/xrpl.js#3318 --- packages/xrpl/HISTORY.md | 1 + packages/xrpl/src/client/connection.ts | 31 ++++++++- packages/xrpl/test/connection.test.ts | 91 ++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index b5b37ddbf1..2a44e058fe 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -33,6 +33,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr * Disallow the input of Authorization Credentials over insecure WebSocket connections (`ws[+unix]?://`) to prevent MITM eavesdropping of sensitive data. * Fix incorrect `MPTAmount` field type to `string` instead of `MPTAmount`. * Fix `Client.getServerInfo()` swallowing errors from the underlying `server_info` request, which left `client.networkID` undefined and caused `autofill()` to silently omit the `NetworkID` field — producing signed transactions valid on the wrong network (cross-network replay risk). The method now throws on request failure or when the response is missing `network_id`. ([#3321](https://github.com/XRPLF/xrpl.js/issues/3321)) +* Refuse to forward server-supplied `data.type` values that collide with `Connection`'s internal state events (`connected`, `disconnected`, `error`, `reconnect`). Previously a malicious or compromised server could inject `{"type":"connected"}` (and friends) over the wire and trigger spoofed local state transitions; the message is now dropped and surfaced as a `badMessage` error. ([#3318](https://github.com/XRPLF/xrpl.js/issues/3318)) ## 4.6.0 (2026-02-12) diff --git a/packages/xrpl/src/client/connection.ts b/packages/xrpl/src/client/connection.ts index 54ad7a288f..2c8dc3d47e 100644 --- a/packages/xrpl/src/client/connection.ts +++ b/packages/xrpl/src/client/connection.ts @@ -22,6 +22,21 @@ const SECONDS_PER_MINUTE = 60 const TIMEOUT = 20 const CONNECTION_TIMEOUT = 5 +/** + * Event names that are emitted internally by Connection to signal local + * client/socket state transitions. These must never be emitted as a result + * of a server-supplied `data.type`, otherwise a malicious or compromised + * rippled server could spoof connection state (e.g. claim the client is + * `connected` when it is not, or inject a fake `error`/`disconnected`/ + * `reconnect` to drive the client into a bad state). See DGE-6740. + */ +const RESERVED_INTERNAL_EVENTS: ReadonlySet = new Set([ + 'connected', + 'disconnected', + 'error', + 'reconnect', +]) + /** * ConnectionOptions is the configuration for the Connection class. */ @@ -354,7 +369,21 @@ export class Connection extends EventEmitter { } if (data.type) { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true - this.emit(data.type as string, data) + const type = data.type as string + // Refuse to forward server-supplied event names that collide with + // Connection's internal state events. Otherwise a rogue server could + // spoof local connection state by sending e.g. `{"type":"connected"}` + // or `{"type":"error"}`. See DGE-6740. + if (RESERVED_INTERNAL_EVENTS.has(type)) { + this.emit( + 'error', + 'badMessage', + `Server message used reserved internal event type "${type}"; dropping.`, + message, + ) + return + } + this.emit(type, data) } if (data.type === 'response') { try { diff --git a/packages/xrpl/test/connection.test.ts b/packages/xrpl/test/connection.test.ts index 085e49be7a..627dde5501 100644 --- a/packages/xrpl/test/connection.test.ts +++ b/packages/xrpl/test/connection.test.ts @@ -903,6 +903,97 @@ describe('Connection', function () { TIMEOUT, ) + // DGE-6740: Server messages whose `type` collides with a reserved internal + // event name (`connected`, `disconnected`, `error`, `reconnect`) must not + // be forwarded as that internal event. Otherwise a rogue server could spoof + // connection state. + it.each(['connected', 'disconnected', 'reconnect'])( + 'drops server message with reserved internal event type "%s"', + async (reservedType) => { + // If the fix regresses, the reserved listener will fire and we'll fail. + const spoofTrigger = new Promise((_resolve, reject) => { + clientContext.client.connection.on(reservedType, () => { + reject( + new XrplError( + `Reserved internal event "${reservedType}" was emitted from a server message`, + ), + ) + }) + }) + + const errorReceived = new Promise((resolve) => { + clientContext.client.connection.on( + 'error', + (errorCode, errorMessage) => { + if ( + errorCode === 'badMessage' && + typeof errorMessage === 'string' && + errorMessage.includes(reservedType) + ) { + resolve() + } + }, + ) + }) + + // @ts-expect-error -- Testing private member + clientContext.client.connection.onMessage( + JSON.stringify({ type: reservedType }), + ) + + await Promise.race([errorReceived, spoofTrigger]) + }, + TIMEOUT, + ) + + // DGE-6740: A spoofed `{"type":"error"}` payload must not be forwarded to + // 'error' listeners as the raw payload. Instead the fix emits an 'error' + // event whose first arg is the string code 'badMessage'. We verify the + // 'error' listener never receives the raw spoofed object. + it( + 'drops server message with reserved internal event type "error"', + async () => { + const spoofedPayload = { type: 'error', error: 'spoof' } + + const result = new Promise((resolve, reject) => { + clientContext.client.connection.on( + 'error', + (errorCodeOrPayload, errorMessage) => { + // The fix's emission: emit('error', 'badMessage', '', rawJson). + if ( + errorCodeOrPayload === 'badMessage' && + typeof errorMessage === 'string' && + errorMessage.includes('error') + ) { + resolve() + return + } + // Anything that looks like the spoofed payload object indicates + // the raw server message reached 'error' listeners, which is the + // exact bug being fixed. + if ( + errorCodeOrPayload != null && + typeof errorCodeOrPayload === 'object' && + (errorCodeOrPayload as { type?: unknown }).type === 'error' + ) { + reject( + new XrplError( + 'Spoofed server payload was forwarded to error listeners', + ), + ) + } + }, + ) + }) + + // @ts-expect-error -- Testing private member + clientContext.client.connection.onMessage(JSON.stringify(spoofedPayload)) + + await result + }, + TIMEOUT, + ) + // it('should clean up websocket connection if error after websocket is opened', async function () { // await clientContext.client.disconnect() // // fail on connection From 4671cb0e0c47045ad990600937fec98f9292147c Mon Sep 17 00:00:00 2001 From: achowdhry-ripple Date: Fri, 22 May 2026 10:28:38 -0400 Subject: [PATCH 2/6] fix(connection): address review feedback on reserved-event guard - Include 'reconnecting' in the reserved internal-event denylist, since Connection also emits it during automatic reconnect attempts. - Drop internal ticket reference from inline comments. - Collapse the separate type:"error" test into the parameterized it.each so all reserved types (including 'error' and 'reconnecting') share one test body. The 'error' iteration retains the extra check that the raw spoofed payload never reaches 'error' listeners. --- packages/xrpl/src/client/connection.ts | 5 +- packages/xrpl/test/connection.test.ts | 79 ++++++++------------------ 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/packages/xrpl/src/client/connection.ts b/packages/xrpl/src/client/connection.ts index 2c8dc3d47e..55f8be91e6 100644 --- a/packages/xrpl/src/client/connection.ts +++ b/packages/xrpl/src/client/connection.ts @@ -28,13 +28,14 @@ const CONNECTION_TIMEOUT = 5 * of a server-supplied `data.type`, otherwise a malicious or compromised * rippled server could spoof connection state (e.g. claim the client is * `connected` when it is not, or inject a fake `error`/`disconnected`/ - * `reconnect` to drive the client into a bad state). See DGE-6740. + * `reconnect`/`reconnecting` to drive the client into a bad state). */ const RESERVED_INTERNAL_EVENTS: ReadonlySet = new Set([ 'connected', 'disconnected', 'error', 'reconnect', + 'reconnecting', ]) /** @@ -373,7 +374,7 @@ export class Connection extends EventEmitter { // Refuse to forward server-supplied event names that collide with // Connection's internal state events. Otherwise a rogue server could // spoof local connection state by sending e.g. `{"type":"connected"}` - // or `{"type":"error"}`. See DGE-6740. + // or `{"type":"error"}`. if (RESERVED_INTERNAL_EVENTS.has(type)) { this.emit( 'error', diff --git a/packages/xrpl/test/connection.test.ts b/packages/xrpl/test/connection.test.ts index 627dde5501..76a803dd60 100644 --- a/packages/xrpl/test/connection.test.ts +++ b/packages/xrpl/test/connection.test.ts @@ -903,59 +903,31 @@ describe('Connection', function () { TIMEOUT, ) - // DGE-6740: Server messages whose `type` collides with a reserved internal - // event name (`connected`, `disconnected`, `error`, `reconnect`) must not - // be forwarded as that internal event. Otherwise a rogue server could spoof - // connection state. - it.each(['connected', 'disconnected', 'reconnect'])( + // Server messages whose `type` collides with a reserved internal event + // name (`connected`, `disconnected`, `error`, `reconnect`, `reconnecting`) + // must not be forwarded as that internal event. Otherwise a rogue server + // could spoof connection state. For `error` specifically, we also verify + // the raw spoofed payload object never reaches 'error' listeners — only + // the synthesized `('error', 'badMessage', ...)` emission is allowed. + it.each(['connected', 'disconnected', 'reconnect', 'reconnecting', 'error'])( 'drops server message with reserved internal event type "%s"', async (reservedType) => { - // If the fix regresses, the reserved listener will fire and we'll fail. - const spoofTrigger = new Promise((_resolve, reject) => { - clientContext.client.connection.on(reservedType, () => { - reject( - new XrplError( - `Reserved internal event "${reservedType}" was emitted from a server message`, - ), - ) - }) - }) - - const errorReceived = new Promise((resolve) => { - clientContext.client.connection.on( - 'error', - (errorCode, errorMessage) => { - if ( - errorCode === 'badMessage' && - typeof errorMessage === 'string' && - errorMessage.includes(reservedType) - ) { - resolve() - } - }, - ) - }) - - // @ts-expect-error -- Testing private member - clientContext.client.connection.onMessage( - JSON.stringify({ type: reservedType }), - ) - - await Promise.race([errorReceived, spoofTrigger]) - }, - TIMEOUT, - ) - - // DGE-6740: A spoofed `{"type":"error"}` payload must not be forwarded to - // 'error' listeners as the raw payload. Instead the fix emits an 'error' - // event whose first arg is the string code 'badMessage'. We verify the - // 'error' listener never receives the raw spoofed object. - it( - 'drops server message with reserved internal event type "error"', - async () => { - const spoofedPayload = { type: 'error', error: 'spoof' } + const spoofedPayload = { type: reservedType, error: 'spoof' } const result = new Promise((resolve, reject) => { + // For non-'error' reserved events, a direct listener on that event + // must never fire from a server message. (We can't attach this for + // 'error' because the fix itself emits 'error' to surface badMessage.) + if (reservedType !== 'error') { + clientContext.client.connection.on(reservedType, () => { + reject( + new XrplError( + `Reserved internal event "${reservedType}" was emitted from a server message`, + ), + ) + }) + } + clientContext.client.connection.on( 'error', (errorCodeOrPayload, errorMessage) => { @@ -963,18 +935,17 @@ describe('Connection', function () { if ( errorCodeOrPayload === 'badMessage' && typeof errorMessage === 'string' && - errorMessage.includes('error') + errorMessage.includes(reservedType) ) { resolve() return } - // Anything that looks like the spoofed payload object indicates - // the raw server message reached 'error' listeners, which is the - // exact bug being fixed. + // If the raw spoofed payload object reaches 'error' listeners, + // the type:"error" spoof bug has regressed. if ( errorCodeOrPayload != null && typeof errorCodeOrPayload === 'object' && - (errorCodeOrPayload as { type?: unknown }).type === 'error' + (errorCodeOrPayload as { type?: unknown }).type === reservedType ) { reject( new XrplError( From b0d323127ce45852d29efc6550a6c01a42dce789 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Mon, 22 Jun 2026 16:44:53 -0700 Subject: [PATCH 3/6] Update packages/xrpl/HISTORY.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- packages/xrpl/HISTORY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index 2a44e058fe..df97c3beb3 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -33,7 +33,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr * Disallow the input of Authorization Credentials over insecure WebSocket connections (`ws[+unix]?://`) to prevent MITM eavesdropping of sensitive data. * Fix incorrect `MPTAmount` field type to `string` instead of `MPTAmount`. * Fix `Client.getServerInfo()` swallowing errors from the underlying `server_info` request, which left `client.networkID` undefined and caused `autofill()` to silently omit the `NetworkID` field — producing signed transactions valid on the wrong network (cross-network replay risk). The method now throws on request failure or when the response is missing `network_id`. ([#3321](https://github.com/XRPLF/xrpl.js/issues/3321)) -* Refuse to forward server-supplied `data.type` values that collide with `Connection`'s internal state events (`connected`, `disconnected`, `error`, `reconnect`). Previously a malicious or compromised server could inject `{"type":"connected"}` (and friends) over the wire and trigger spoofed local state transitions; the message is now dropped and surfaced as a `badMessage` error. ([#3318](https://github.com/XRPLF/xrpl.js/issues/3318)) +* Refuse to forward server-supplied `data.type` values that collide with `Connection`'s internal state events (`connected`, `disconnected`, `error`, `reconnect`, `reconnecting`). Previously a malicious or compromised server could inject `{"type":"connected"}` (and friends) over the wire and trigger spoofed local state transitions; the message is now dropped and surfaced as a `badMessage` error. ([`#3318`](https://github.com/XRPLF/xrpl.js/issues/3318)) ## 4.6.0 (2026-02-12) From 5dfb28d427299bfa1c63656ca73712a97fd13ce2 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Mon, 22 Jun 2026 17:00:01 -0700 Subject: [PATCH 4/6] fix(connection): reject non-string server event types A non-string `type` (e.g. `["error"]`) bypassed the reserved-event Set check but still coerced to a reserved name when passed to `emit`. Validate `type` is a string before forwarding and drop non-strings as `badMessage`. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/xrpl/src/client/connection.ts | 63 +++++++++++++++++++------- packages/xrpl/test/connection.test.ts | 48 ++++++++++++++++++++ 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/packages/xrpl/src/client/connection.ts b/packages/xrpl/src/client/connection.ts index 55f8be91e6..7f8b43a731 100644 --- a/packages/xrpl/src/client/connection.ts +++ b/packages/xrpl/src/client/connection.ts @@ -369,22 +369,7 @@ export class Connection extends EventEmitter { return } if (data.type) { - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true - const type = data.type as string - // Refuse to forward server-supplied event names that collide with - // Connection's internal state events. Otherwise a rogue server could - // spoof local connection state by sending e.g. `{"type":"connected"}` - // or `{"type":"error"}`. - if (RESERVED_INTERNAL_EVENTS.has(type)) { - this.emit( - 'error', - 'badMessage', - `Server message used reserved internal event type "${type}"; dropping.`, - message, - ) - return - } - this.emit(type, data) + this.forwardServerMessage(data, message) } if (data.type === 'response') { try { @@ -399,6 +384,52 @@ export class Connection extends EventEmitter { } } + /** + * Forwards a server message to listeners keyed by its `data.type`, after + * validating the server-controlled `type`. Messages whose `type` is not a + * string, or whose `type` collides with one of Connection's reserved + * internal event names, are dropped and surfaced as a `badMessage` error + * rather than forwarded. + * + * @param data - The parsed server message. + * @param message - The raw message string, used for diagnostics. + */ + private forwardServerMessage( + data: Record, + message: string, + ): void { + // `data.type` is server-controlled, so its runtime type must be checked + // before it is used as an event name. A compile-time `as string` is not + // enough: a payload like `{"type":["error"]}` slips past + // `RESERVED_INTERNAL_EVENTS.has(...)` (an array never matches a string in + // the Set) yet still coerces to the string "error" when passed to `emit`, + // re-opening the spoofing hole. Reject any non-string `type` outright. + if (typeof data.type !== 'string') { + this.emit( + 'error', + 'badMessage', + `Server message 'type' must be a string; dropping.`, + message, + ) + return + } + const type = data.type + // Refuse to forward server-supplied event names that collide with + // Connection's internal state events. Otherwise a rogue server could + // spoof local connection state by sending e.g. `{"type":"connected"}` + // or `{"type":"error"}`. + if (RESERVED_INTERNAL_EVENTS.has(type)) { + this.emit( + 'error', + 'badMessage', + `Server message used reserved internal event type "${type}"; dropping.`, + message, + ) + return + } + this.emit(type, data) + } + /** * Handler for what to do once the connection to the server is open. * diff --git a/packages/xrpl/test/connection.test.ts b/packages/xrpl/test/connection.test.ts index 76a803dd60..152be3c4b6 100644 --- a/packages/xrpl/test/connection.test.ts +++ b/packages/xrpl/test/connection.test.ts @@ -965,6 +965,54 @@ describe('Connection', function () { TIMEOUT, ) + // A non-string `type` must never be used as an event name. Arrays are the + // dangerous case: `{"type":["error"]}` fails the RESERVED_INTERNAL_EVENTS + // Set check (an array is never === a string) but coerces to the string + // "error" when passed to `emit`, which would otherwise re-open the spoofing + // hole the reserved-event guard closes. All non-string types are dropped and + // surfaced as `badMessage`. + it.each([ + ['connected', ['connected']], + ['disconnected', ['disconnected']], + ['reconnect', [['reconnect']]], + ['error', ['error']], + ])( + 'drops server message whose non-string type coerces to "%s"', + async (coercesTo, spoofedType) => { + const spoofedPayload = { type: spoofedType, error: 'spoof' } + + const result = new Promise((resolve, reject) => { + // Regression guard: the event the non-string type coerces to must never + // receive the raw server payload. Keying off the forwarded payload + // object (rather than the event merely firing) lets this same check + // cover the `error` case, where the event is also the channel the fix + // uses to report `badMessage`. + clientContext.client.connection.on(coercesTo, (forwarded) => { + if (forwarded != null && typeof forwarded === 'object') { + reject( + new XrplError( + `Non-string server type was forwarded as internal event "${coercesTo}"`, + ), + ) + } + }) + + // Success: the message is dropped and surfaced as a badMessage error. + clientContext.client.connection.on('error', (errorCode) => { + if (errorCode === 'badMessage') { + resolve() + } + }) + }) + + // @ts-expect-error -- Testing private member + clientContext.client.connection.onMessage(JSON.stringify(spoofedPayload)) + + await result + }, + TIMEOUT, + ) + // it('should clean up websocket connection if error after websocket is opened', async function () { // await clientContext.client.disconnect() // // fail on connection From 8b7434065e8dbef4ebb8f822820e5c2bf172c3ae Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:59:55 -0700 Subject: [PATCH 5/6] Update packages/xrpl/test/connection.test.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- packages/xrpl/test/connection.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/xrpl/test/connection.test.ts b/packages/xrpl/test/connection.test.ts index 152be3c4b6..a8fcc88670 100644 --- a/packages/xrpl/test/connection.test.ts +++ b/packages/xrpl/test/connection.test.ts @@ -975,6 +975,7 @@ describe('Connection', function () { ['connected', ['connected']], ['disconnected', ['disconnected']], ['reconnect', [['reconnect']]], + ['reconnecting', ['reconnecting']], ['error', ['error']], ])( 'drops server message whose non-string type coerces to "%s"', From bb70c6dde7b12bcb212831215da8de448d207a90 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Wed, 24 Jun 2026 16:04:12 -0700 Subject: [PATCH 6/6] fix(connection): improve non-string type diagnostics and test fail-fast - Include the offending `typeof data.type` in the non-string `badMessage` error to aid debugging. - Fail fast on unexpected `error` emissions in the reserved-event test instead of waiting out the 20s timeout. Addresses review feedback on #3378. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/xrpl/src/client/connection.ts | 2 +- packages/xrpl/test/connection.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/xrpl/src/client/connection.ts b/packages/xrpl/src/client/connection.ts index 7f8b43a731..1beed84411 100644 --- a/packages/xrpl/src/client/connection.ts +++ b/packages/xrpl/src/client/connection.ts @@ -408,7 +408,7 @@ export class Connection extends EventEmitter { this.emit( 'error', 'badMessage', - `Server message 'type' must be a string; dropping.`, + `Server message 'type' must be a string (got ${typeof data.type}); dropping.`, message, ) return diff --git a/packages/xrpl/test/connection.test.ts b/packages/xrpl/test/connection.test.ts index a8fcc88670..aca7ff2f28 100644 --- a/packages/xrpl/test/connection.test.ts +++ b/packages/xrpl/test/connection.test.ts @@ -952,7 +952,15 @@ describe('Connection', function () { 'Spoofed server payload was forwarded to error listeners', ), ) + return } + // Any other 'error' emission is unexpected — fail fast instead of + // waiting for the test to time out. + reject( + new XrplError( + `Unexpected 'error' emission: ${JSON.stringify(errorCodeOrPayload)}`, + ), + ) }, ) })