Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 (got ${typeof data.type}); 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.
*
Expand Down
119 changes: 119 additions & 0 deletions packages/xrpl/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,125 @@ 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',
),
)
return
}
Comment thread
cybele-ripple marked this conversation as resolved.
// 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)}`,
),
)
},
)
})

// @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']]],
['reconnecting', ['reconnecting']],
['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