Skip to content

Fix heap-buffer-overflow in CLUSTER FORGET with invalid node ID length#14417

Merged
moticless merged 1 commit intoredis:unstablefrom
moticless:fix-cluster-forget-bad-node-id
Oct 9, 2025
Merged

Fix heap-buffer-overflow in CLUSTER FORGET with invalid node ID length#14417
moticless merged 1 commit intoredis:unstablefrom
moticless:fix-cluster-forget-bad-node-id

Conversation

@moticless
Copy link
Collaborator

Description

Fix a heap-buffer-overflow vulnerability in the CLUSTER FORGET command when provided with a node ID shorter than the expected 40 bytes.

The Issue

When CLUSTER FORGET is called with a node ID that has a length smaller than CLUSTER_NAMELEN (40 bytes), the clusterBlacklistExists() function would read beyond the allocated string buffer. This occurs because the function always attempted to read exactly 40 bytes via sdsnewlen(nodeid, CLUSTER_NAMELEN).

This primarily impacts memory inspection tools (e.g., AddressSanitizer) and could potentially cause issues in production environments.

Changes

  • Added a size_t len parameter to clusterBlacklistExists() to use the actual string length
  • Updated all call sites to pass the appropriate length
  • Added a test case to verify the fix
  • Test added to verify that CLUSTER FORGET with an invalid short node ID returns an appropriate error.

This PR is based on:
valkey-io/valkey#2108

Co-authored-by: Ran Shidlansik [email protected]

When CLUSTER FORGET is called with a node ID shorter than 40 bytes,
clusterBlacklistExists would read beyond the allocated string buffer
because it always read CLUSTER_NAMELEN (40) bytes regardless of the
actual string length provided.

This fix adds a length parameter to clusterBlacklistExists() to use
the actual length of the provided string, preventing the buffer overflow.

This issue was discovered and fixed in Valkey PR redis#2108. The same
vulnerability exists in Redis and this commit applies the equivalent fix.

Co-authored-by: Ran Shidlansik <[email protected]>
@moticless moticless merged commit 48d0aa9 into redis:unstable Oct 9, 2025
19 checks passed
@sundb sundb added the release-notes indication that this issue needs to be mentioned in the release notes label Oct 10, 2025
@sundb sundb moved this from Todo to Done in Redis 8.4 Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants