Skip to content

fix use-after-free on next_sub in broker fan-out loops#556

Open
jmestwa-coder wants to merge 1 commit into
wolfSSL:masterfrom
jmestwa-coder:broker-fanout-next-sub-uaf
Open

fix use-after-free on next_sub in broker fan-out loops#556
jmestwa-coder wants to merge 1 commit into
wolfSSL:masterfrom
jmestwa-coder:broker-fanout-next-sub-uaf

Conversation

@jmestwa-coder

Copy link
Copy Markdown

Use-after-free in the two broker subscription fan-out loops when a WebSocket subscriber's peer closes during a fan-out write:

  • BrokerHandle_Publish and BrokerClient_PublishWillImmediate snapshot next_sub, but the write to a WS subscriber spins lws_service and a re-entrant LWS_CALLBACK_CLOSED runs BrokerSubs_RemoveClient, freeing that subscriber's BrokerSub nodes
  • pending_remove keeps the BrokerClient alive across the spin, but the sub nodes are freed regardless, so a subscriber with two adjacent matching filters has its snapshotted next_sub freed under the loop, then sub = next_sub dereferences it
  • revalidate next_sub against broker->subs after the write, the same walk MqttBroker_Step already does for the client list
  • static-memory builds use array indices and are unaffected

Both subscription fan-out loops snapshot next_sub before a write that can drive a re-entrant WebSocket close. That close runs BrokerSubs_RemoveClient and frees the subscriber's sub nodes, while pending_remove only defers the client struct, so next_sub itself can be freed. Revalidate it against broker->subs after the write, like MqttBroker_Step does for the client list.
@wolfSSL-Bot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@dgarske

dgarske commented Jun 22, 2026

Copy link
Copy Markdown
Member

Hi @jmestwa-coder , thank you for this. I see your CLA in process in ZD 21916. Assigning this over to @embhorn to manage. Thanks, David Garske, wolfSSL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants