Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/xrpl/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`, `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)
Expand Down
65 changes: 63 additions & 2 deletions packages/xrpl/src/client/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@ 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`/`reconnecting` to drive the client into a bad state).
*/
const RESERVED_INTERNAL_EVENTS: ReadonlySet<string> = new Set([
'connected',
'disconnected',
'error',
'reconnect',
'reconnecting',
])

/**
* ConnectionOptions is the configuration for the Connection class.
*/
Expand Down Expand Up @@ -353,8 +369,7 @@ export class Connection extends EventEmitter {
return
}
if (data.type) {
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
this.emit(data.type as string, data)
this.forwardServerMessage(data, message)
}
if (data.type === 'response') {
try {
Expand All @@ -369,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<string, unknown>,
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.`,
Comment thread
cybele-ripple marked this conversation as resolved.
Outdated
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.
*
Expand Down
110 changes: 110 additions & 0 deletions packages/xrpl/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,116 @@ describe('Connection', function () {
TIMEOUT,
)

// 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) => {
const spoofedPayload = { type: reservedType, error: 'spoof' }

const result = new Promise<void>((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) => {
// The fix's emission: emit('error', 'badMessage', '<msg>', rawJson).
if (
errorCodeOrPayload === 'badMessage' &&
typeof errorMessage === 'string' &&
errorMessage.includes(reservedType)
) {
resolve()
return
}
// 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 === reservedType
) {
reject(
new XrplError(
'Spoofed server payload was forwarded to error listeners',
),
)
}
Comment thread
cybele-ripple marked this conversation as resolved.
},
)
})

// @ts-expect-error -- Testing private member
clientContext.client.connection.onMessage(JSON.stringify(spoofedPayload))

await result
},
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']],
])(
Comment thread
ckeshava marked this conversation as resolved.
'drops server message whose non-string type coerces to "%s"',
async (coercesTo, spoofedType) => {
const spoofedPayload = { type: spoofedType, error: 'spoof' }

const result = new Promise<void>((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
Expand Down
Loading