Fix cluster bus self-forget use-after-free - #4675
roshkhatri wants to merge 1 commit into
Conversation
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe cluster bus now rejects ChangesCluster self-forget validation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to The malformed self-forget packet is rejected while valid node state remains available. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| } else if (type == CLUSTERMSG_EXT_TYPE_FORGOTTEN_NODE) { | ||
| clusterMsgPingExtForgottenNode *forgotten_node_ext = &(ext->ext[0].forgotten_node); | ||
| clusterNode *n = clusterLookupNode(forgotten_node_ext->name, CLUSTER_NAMELEN); | ||
| if (n == sender) { |
There was a problem hiding this comment.
How does this message originate though?
There was a problem hiding this comment.
This should be a report from a security email? I recall seeing it. Could we just ignore it by adding a condition like n != sender below? (both are ok to me, not a blocker)
This PR fixes a use-after-free in the handling of
CLUSTERMSG_EXT_TYPE_FORGOTTEN_NODEcluster-bus PING extensions.A cluster node can use this extension to tell its peers that another node has been forgotten. However, the receiver did not check whether the node named in the extension was also the sender of the message currently being processed.
In that case,
clusterDelNode()deleted the sender and freed its active cluster link and receive buffer. The extension handler then continued processing the same message and later accessed the now-freed sender and link. This could cause memory corruption and crash the server.How is it fixed?
A node should not be able to legitimately announce that it has forgotten itself. The handler now treats this as an invalid cluster-bus message:
This preserves the existing behavior for valid forgotten-node extensions.