Merge bd679c0668 into 30f57f32bd

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;
@@ -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);
             }

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
+        }
+    }
+}
 }