forked from KickTalkOrg/KickTalk
-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Labels
Description
Summary
WebSocket connection and subscription logic in SharedKickPusher is not correlated to a specific socket and not serialized. Stale events and concurrent subscribe/unsubscribe paths can corrupt connection state, create duplicate or orphaned subscriptions, and lead to message amplification or loss.
Severity: Medium • CWE-362 (Race Condition)
Affected component
utils/services/kick/sharedKickPusher.js
Precise code references
addChatroom()subscribes immediately while other paths may also subscribe- 36–54
connect()state gating lacks per-socket correlation- 69–79, 93–104
openhandler updates global state without verifying current socket- 115–145
closehandler unconditionally resets global state; stale close can clobber new connection- 177–227
messagehandler forpusher:connection_establishedsetsconnected,socketId, then subscribes (no socket correlation)- 229–261
subscribeToAllChannels()runs multiple async operations without serialization- 333–346
subscribeToUserEvents()has a duplicate-subscription window beforeuserEventsSubscribedflips- 348–381
subscribeToChatroomChannels()TOCTOU onsubscribedChannelsSet allowing duplicate subscribe- 383–401
unsubscribeFromChatroomChannels()may race with subscribe- 433–446
Impact
- Connection state corruption (e.g.,
connectionStatebecomesdisconnecteddue to a stale close while a new socket is live). - Duplicate or orphaned channel subscriptions:
- Duplicate event handling and message amplification.
- Orphaned subscriptions missing events or leaking processing.
- Potential DoS via rapid reconnect/subscription churn.
Evidence
- Event handlers (
open,close,message) update shared state without checking they belong to the currentthis.chatinstance. - Concurrent subscription flows:
addChatroom()triggerssubscribeToChatroomChannels()whilesubscribeToAllChannels()is also iterating, both checking!this.subscribedChannels.has(channel)before adding, enabling duplicates.
- No serialization of async operations across
subscribeToAllChannels(),subscribeToUserEvents(), andsubscribeToChatroomChannels().
PoCs in repo
exploits/poc_websocket_race_conditions.pyexploits/poc_websocket_race_demo.js
Reproduction (one approach)
- Trigger multiple connection attempts quickly:
- Call
connect()multiple times in rapid succession.
- Call
- While connecting, rapidly add/remove chatrooms to create concurrent subscribe/unsubscribe:
addChatroom()/removeChatroom()in quick intervals.
- Force a close on the old socket (network blip), observe stale
closeresetting state for the new socket. - Observe logs:
- Duplicated
pusher:subscribemessages. connectionStatetoggling inconsistently.- Orphaned channels not present in
chatroomsmap.
- Duplicated
Recommended mitigations
- Per-connection correlation
- Maintain a monotonically increasing
connectionIdfor eachconnect(). - Capture
connectionId(or socket ref) in all bound handlers and early-return if it does not match the current connection. - Track and remove handlers when replacing the socket.
- Maintain a monotonically increasing
- Serialize state transitions
- Use an async mutex/semaphore around
connect()and subscription flows. - Queue subscription/unsubscription operations; collapse duplicates and process in order.
- Use an async mutex/semaphore around
- Idempotency guards
- Use
pendingSubscriptions/pendingUnsubsin addition tosubscribedChannelsto avoid TOCTOU duplicates.
- Use
- Robust FSM
- Constrain permissible transitions (disconnected→connecting→connected) and drop stale events.
CWE / Severity
- CWE-362: Race Condition
- Severity: Medium
Environment
- Observed in repo head at time of filing; PoCs demonstrate behavior against public Pusher endpoint.
Reactions are currently unavailable