MOD-15749 Fix heap-buffer-overflow on short-form CLUSTERSET node IDs

[gabsow]

Summary

Fix a heap-buffer-overflow caught by ASan in the short-form CLUSTERSET path. The short-form caller was passing node IDs from RedisModule_GetClusterNodesList (40 bytes, no null terminator — see redis core cluster_legacy.c's getClusterNodesList) straight into MR_CreateNode, which treats the id as a C string downstream.

Root cause

Bug introduced by #92 (the short-form path); not related to #94. Before #92, MR_CreateNode's only caller was SetClusterDataLongForm, which passes a properly null-terminated realId[REDISMODULE_NODE_ID_LEN + 1] stack buffer. #92 added a second caller — SetClusterDataShortForm — that violates that contract.

The bug surfaced now (rather than at #92's merge time) because the strengthened test_short_form_clusterset on RedisTimeSeries#2034 is the first test to exercise the buggy path under sanitizer build.

ASan output (canonical)

==27487==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x504000016078
READ of size 41 at 0x504000016078 thread T4 (timeseries-el)
    #0 __interceptor_strlen
    #1 zstrdup_usable      (redis/src/zmalloc.c:647)
    #2 zstrdup             (redis/src/zmalloc.c:655)
    #3 RM_Strdup           (redis/src/module.c:585)
    #4 MR_CreateNode       (LibMR/src/cluster.c:844)
    #5 SetClusterDataShortForm (LibMR/src/cluster.c:1153)
    ...

0x504000016078 is located 0 bytes to the right of 40-byte region
allocated by thread T4 here:
    #0 __interceptor_malloc
    #2 zmalloc                       (redis/src/zmalloc.c:285)
    #3 getClusterNodesList           (redis/src/cluster_legacy.c:5865)
    #4 SetClusterDataShortForm       (LibMR/src/cluster.c:1127)

Fix (minimal, single-caller)

Scoped strictly to the buggy caller, SetClusterDataShortForm. At the top of the per-node loop, copy nodeList[i] into a null-terminated stack buffer (REDISMODULE_NODE_ID_LEN + 1 bytes), then use that buffer for the rest of the iteration. MR_CreateNode is byte-identical to master. SetClusterDataLongForm is byte-identical to master.

Side benefit: also covers the latent RedisModule_Log("...%s...", nodeId) printf %s on the GetClusterNodeInfo failure branch, which would have the same overflow class. GetClusterNodeInfo and GetClusterNodeSlotRanges themselves are unaffected (redis core implements them with fixed-length clusterLookupNode(nodeid, CLUSTER_NAMELEN)).

Risk

Very low. ~9 added lines (a stack buffer + memcpy + null-terminator + a comment block), in exactly one function. Long-form path completely untouched. MR_CreateNode's contract is unchanged — callers are still expected to pass a null-terminated id; the fix makes the buggy caller honor that contract instead of changing the contract.

Test plan

• LibMR builds clean locally with the patch.
• Downstream sanitizer CI on RedisTimeSeries#2034 — after re-pinning deps/LibMR to this branch HEAD, the linux-sanitizer / x64-jammy job should go from red to green.

---

Note

Low Risk
Localized change in one loop in SetClusterDataShortForm; long-form path and MR_CreateNode contract unchanged.

Overview
Fixes a heap-buffer-overflow in short-form CLUSTERSET (SetClusterDataShortForm) when node IDs from RedisModule_GetClusterNodesList (fixed 40 bytes, not null-terminated) were passed where C strings are expected.

Each list entry is now copied into a REDISMODULE_NODE_ID_LEN + 1 stack buffer with an explicit '\0' before RedisModule_GetClusterNodeInfo, logging, and MR_CreateNode (which duplicates the id via string APIs). Long-form CLUSTERSET and MR_CreateNode itself are unchanged.

^{Reviewed by Cursor Bugbot for commit 8851627. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7cc1387. Configure here.}