From 8a1d36abbe68cbc04b4f9b0ce07d415a2970885c Mon Sep 17 00:00:00 2001 From: Sergey Georgiev Date: Tue, 26 May 2026 09:27:08 +0300 Subject: [PATCH 1/2] fixed: missing null-terminator validation in cluster hostname/nodename extensions --- src/cluster_legacy.c | 12 ++++ tests/unit/cluster/hostnames.tcl | 97 ++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index c93aea2ad..7abc40de8 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -2831,6 +2831,18 @@ int clusterProcessPacket(clusterLink *link) { (unsigned long long) totlen); return 1; } + uint16_t exttype = ntohs(ext->type); + if (exttype == CLUSTERMSG_EXT_TYPE_HOSTNAME || + exttype == CLUSTERMSG_EXT_TYPE_HUMAN_NODENAME) { + uint32_t datalen = extlen - sizeof(clusterMsgPingExt); + char *str = (char *) ext->ext; + if (datalen == 0 || str[datalen - 1] != '\0') { + serverLog(LL_WARNING, + "Received %s packet with missing null terminator in extension type %d", + clusterGetMessageTypeString(type), exttype); + return 1; + } + } explen += extlen; ext = getNextPingExt(ext); } diff --git a/tests/unit/cluster/hostnames.tcl b/tests/unit/cluster/hostnames.tcl index 223622864..b48a0b7bc 100644 --- a/tests/unit/cluster/hostnames.tcl +++ b/tests/unit/cluster/hostnames.tcl @@ -16,6 +16,63 @@ proc get_slot_field {slot_output shard_id node_id attrib_id} { return [lindex [lindex [lindex $slot_output $shard_id] $node_id] $attrib_id] } +proc build_cluster_bus_ping {sender_name sender_port sender_cport extensions_data} { + set CLUSTER_NAMELEN 40 + set NET_IP_STR_LEN 46 + set CLUSTERMSG_TYPE_PING 0 + set CLUSTERMSG_FLAG0_EXT_DATA 0x04 + + set num_extensions [expr {[string length $extensions_data] > 0 ? 1 : 0}] + set mflags0 [expr {$num_extensions > 0 ? $CLUSTERMSG_FLAG0_EXT_DATA : 0}] + set totlen [expr {2256 + [string length $extensions_data]}] + + set sender_padded [binary format a${CLUSTER_NAMELEN} $sender_name] + set myslots [string repeat \x00 [expr {16384/8}]] + set slaveof [string repeat \x00 $CLUSTER_NAMELEN] + set myip [string repeat \x00 $NET_IP_STR_LEN] + set notused1 [string repeat \x00 30] + + set hdr "" + append hdr "RCmb" + append hdr [binary format I $totlen] + append hdr [binary format S 1] + append hdr [binary format S $sender_port] + append hdr [binary format S $CLUSTERMSG_TYPE_PING] + append hdr [binary format S 0] + append hdr [binary format W 1] + append hdr [binary format W 2] + append hdr [binary format W 0] + append hdr $sender_padded + append hdr $myslots + append hdr $slaveof + append hdr $myip + append hdr [binary format S $num_extensions] + append hdr $notused1 + append hdr [binary format S 0] + append hdr [binary format S $sender_cport] + append hdr [binary format S 1] + append hdr [binary format c 0] + append hdr [binary format ccc $mflags0 0 0] + + append hdr $extensions_data + return $hdr +} + +proc build_hostname_extension {hostname_bytes} { + set ext_header_size 8 + set total_ext_len [expr {$ext_header_size + [string length $hostname_bytes]}] + set padded_len [expr {(($total_ext_len + 7) / 8) * 8}] + set padding_len [expr {$padded_len - $total_ext_len}] + + set ext "" + append ext [binary format I $padded_len] + append ext [binary format S 0] + append ext [binary format S 0] + append ext $hostname_bytes + append ext [string repeat \x00 $padding_len] + return $ext +} + # Start a cluster with 3 masters and 4 replicas. # These tests rely on specific node ordering, so make sure no node fails over. start_cluster 3 4 {tags {external:skip cluster} overrides {cluster-replica-no-failover yes}} { @@ -227,4 +284,44 @@ test "Test hostname validation" { # Note this isn't a valid hostname, but it passes our internal validation R 0 config set cluster-announce-hostname "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-." } + +test "PING with hostname extension missing null terminator is rejected" { + for {set j 0} {$j < [llength $::servers]} {incr j} { + R $j config set cluster-announce-hostname "" + } + wait_for_condition 50 100 { + [are_hostnames_propagated ""] eq 1 + } else { + fail "hostnames were not cleared" + } + + set node1_id [R 1 CLUSTER MYID] + set target_host [srv 0 host] + set target_bus_port [expr {[srv 0 port] + 10000}] + set sender_port [srv -1 port] + set sender_cport [expr {$sender_port + 10000}] + + # Send a PING with a HOSTNAME extension that has no null terminator. + set bad_hostname [string repeat A 32] + set bad_ext [build_hostname_extension $bad_hostname] + set bad_packet [build_cluster_bus_ping $node1_id $sender_port $sender_cport $bad_ext] + + set fd [socket $target_host $target_bus_port] + fconfigure $fd -translation binary -buffering full + puts -nonewline $fd $bad_packet + flush $fd + after 500 + close $fd + + # The fix rejects the packet and logs a warning. + wait_for_log_messages 0 {"*missing null terminator*"} 0 20 500 + + # Verify the hostname was NOT updated to the malformed value. + set nodes_output [R 0 CLUSTER NODES] + foreach line [split $nodes_output "\n"] { + if {[string match "$node1_id *" $line]} { + assert_no_match "*AAAA*" $line + } + } +} } From bd679c066859ec6f9a60c249a8c78ddac097596a Mon Sep 17 00:00:00 2001 From: Sergey Georgiev Date: Tue, 26 May 2026 11:24:18 +0300 Subject: [PATCH 2/2] fixed: issue from review --- src/cluster_legacy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c index 7abc40de8..dfbbd384d 100644 --- a/src/cluster_legacy.c +++ b/src/cluster_legacy.c @@ -2820,7 +2820,7 @@ int clusterProcessPacket(clusterLink *link) { clusterMsgPingExt *ext = getInitialPingExt(hdr, count); while (extensions--) { uint16_t extlen = getPingExtLength(ext); - if (extlen % 8 != 0) { + if (extlen < sizeof(clusterMsgPingExt) || extlen % 8 != 0) { serverLog(LL_WARNING, "Received a %s packet without proper padding (%d bytes)", clusterGetMessageTypeString(type), (int) extlen); return 1;