From 383a45a82f4cf90b004388fb9ebf3f715809d27c Mon Sep 17 00:00:00 2001 From: Ashray Chowdhry Date: Mon, 18 May 2026 09:01:04 -0400 Subject: [PATCH 1/2] fix(connection): block reserved internal event types from server messages (DGE-6740) 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 f2b79844b0..0dcca8cc27 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -16,6 +16,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 6b41279c2700954f71cad9b48dcdcccf98d1fe7f Mon Sep 17 00:00:00 2001 From: achowdhry-ripple Date: Fri, 22 May 2026 10:28:38 -0400 Subject: [PATCH 2/2] 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(