Fix heap-buffer-overflow in CLUSTER FORGET with invalid node ID length#14417
Merged
moticless merged 1 commit intoOct 9, 2025
Merged
Conversation
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 <ranshid@amazon.com>
sundb
approved these changes
Oct 9, 2025
sundb
pushed a commit
to sundb/redis
that referenced
this pull request
Jun 4, 2026
redis#14417) Fix a heap-buffer-overflow vulnerability in the `CLUSTER FORGET` command when provided with a node ID shorter than the expected 40 bytes. 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)`. 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 <ranshid@amazon.com> (cherry picked from commit 48d0aa9)
sundb
pushed a commit
to sundb/redis
that referenced
this pull request
Jun 4, 2026
redis#14417) Fix a heap-buffer-overflow vulnerability in the `CLUSTER FORGET` command when provided with a node ID shorter than the expected 40 bytes. 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)`. 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 <ranshid@amazon.com> (cherry picked from commit 48d0aa9)
Merged
sundb
pushed a commit
that referenced
this pull request
Jun 4, 2026
#14417) Fix a heap-buffer-overflow vulnerability in the `CLUSTER FORGET` command when provided with a node ID shorter than the expected 40 bytes. 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)`. 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 <ranshid@amazon.com> (cherry picked from commit 48d0aa9)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix a heap-buffer-overflow vulnerability in the
CLUSTER FORGETcommand when provided with a node ID shorter than the expected 40 bytes.The Issue
When
CLUSTER FORGETis called with a node ID that has a length smaller thanCLUSTER_NAMELEN(40 bytes), theclusterBlacklistExists()function would read beyond the allocated string buffer. This occurs because the function always attempted to read exactly 40 bytes viasdsnewlen(nodeid, CLUSTER_NAMELEN).This primarily impacts memory inspection tools (e.g., AddressSanitizer) and could potentially cause issues in production environments.
Changes
size_t lenparameter toclusterBlacklistExists()to use the actual string lengthCLUSTER FORGETwith an invalid short node ID returns an appropriate error.This PR is based on:
valkey-io/valkey#2108
Co-authored-by: Ran Shidlansik ranshid@amazon.com