Fix cluster AUX-field newline/control-character injection bricks a node on restart

* fix cluster AUX-field newline/control-character injection bricks a node on restart

src/cluster.c

@@ -805,7 +805,12 @@ int verifyClusterNodeId(const char *name, int length) {
 }
 
 int isValidAuxChar(int c) {
-    return isalnum(c) || (strchr("!#$%&()*+:;<>?@[]^{|}~", c) == NULL);
+    /* Reject control characters (0x00-0x1F and 0x7F). */
+    if (iscntrl(c)) {
+        return 0;
+    }
+    /* Reject forbidden characters including nodes.conf delimiters and special parsing characters */
+    return isalnum(c) || (strchr("!#$%&()*+:;<>?@[]^{|}~,= \"'\\", c) == NULL);
 }
 
 int isValidAuxString(char *s, unsigned int length) {

src/config.c

@@ -24,6 +24,7 @@
 #include <string.h>
 #include <locale.h>
 #include <ctype.h>
+#include <arpa/inet.h>
 
 /*-----------------------------------------------------------------------------
  * Config file name-value maps.
@@ -2452,6 +2453,23 @@ static int isValidAnnouncedHostname(char *val, const char **err) {
     return 1;
 }
 
+/* Validation function for cluster-announce-ip.
+ * Ensures the IP address is valid and rejects control characters. */
+static int isValidClusterAnnounceIp(char *val, const char **err) {
+    unsigned char buf[sizeof(struct in6_addr)];
+    /* Empty string is allowed - it will be converted to NULL by EMPTY_STRING_IS_NULL flag */
+    if (val[0] == '\0') {
+        return 1;
+    }
+
+    if (inet_pton(AF_INET, val, buf) != 1 &&
+        inet_pton(AF_INET6, val, buf) != 1) {
+        *err = "Cluster announce IP must be a valid IPv4 or IPv6 address";
+        return 0;
+    }
+    return 1;
+}
+
 /* Validate specified string is a valid proc-title-template */
 static int isValidProcTitleTemplate(char *val, const char **err) {
     if (!validateProcTitleTemplate(val)) {
@@ -3159,7 +3177,7 @@ standardConfig static_configs[] = {
     createStringConfig("pidfile", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.pidfile, NULL, NULL, NULL),
     createStringConfig("replica-announce-ip", "slave-announce-ip", MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.slave_announce_ip, NULL, NULL, NULL),
     createStringConfig("masteruser", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.masteruser, NULL, NULL, NULL),
-    createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, updateClusterIp),
+    createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, isValidClusterAnnounceIp, updateClusterIp),
     createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", NULL, NULL),
     createStringConfig("cluster-announce-hostname", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_hostname, NULL, isValidAnnouncedHostname, updateClusterHostname),
     createStringConfig("cluster-announce-human-nodename", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_human_nodename, NULL, isValidAnnouncedNodename, updateClusterHumanNodename),

tests/unit/cluster/announced-endpoints.tcl

@@ -72,4 +72,88 @@ start_cluster 2 2 {tags {external:skip cluster}} {
             fail "Cluster announced port was not updated in cluster slots"
         }
     }
+
+    # Tests for cluster-announce-ip validation
+    test "cluster-announce-ip validation" {
+        catch {R 0 config set cluster-announce-ip "192.168.1.100\nnext"} err
+        assert_match "*valid IPv4 or IPv6*" $err
+
+        catch {R 0 config set cluster-announce-ip "10.0.0.1\ttab"} err
+        assert_match "*valid IPv4 or IPv6*" $err
+
+        catch {R 0 config set cluster-announce-ip "1.2.3.4\r\n"} err
+        assert_match "*valid IPv4 or IPv6*" $err
+
+        catch {R 0 config set cluster-announce-ip "redis-node-1.example.com"} err
+        assert_match "*valid IPv4 or IPv6*" $err
+
+        catch {R 0 config set cluster-announce-ip "192.168.1"} err
+        assert_match "*valid IPv4 or IPv6*" $err
+
+        # Accept valid IPv4
+        R 0 config set cluster-announce-ip "192.168.1.100"
+        assert_equal "192.168.1.100" [lindex [R 0 config get cluster-announce-ip] 1]
+
+        # Accept valid IPv6
+        R 0 config set cluster-announce-ip "2001:db8::1"
+        assert_equal "2001:db8::1" [lindex [R 0 config get cluster-announce-ip] 1]
+
+        # Can be cleared
+        R 0 config set cluster-announce-ip ""
+        assert_equal "" [lindex [R 0 config get cluster-announce-ip] 1]
+    }
+
+    # Tests for cluster-announce-human-nodename validation
+    test "cluster-announce-human-nodename validation" {
+        # Reject control characters
+        catch {R 0 config set cluster-announce-human-nodename "badchar\nnext"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad\ttab"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad\r\nline"} err
+        assert_match "*invalid character*" $err
+
+        # Reject delimiter characters (comma, equals, space)
+        catch {R 0 config set cluster-announce-human-nodename "bad,comma"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad=equals"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad space"} err
+        assert_match "*invalid character*" $err
+
+        # Reject quote characters (double quote, single quote, backslash)
+        catch {R 0 config set cluster-announce-human-nodename "bad\"quote"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad'quote"} err
+        assert_match "*invalid character*" $err
+
+        catch {R 0 config set cluster-announce-human-nodename "bad\\slash"} err
+        assert_match "*invalid character*" $err
+
+        # Accept valid names
+        R 0 config set cluster-announce-human-nodename "my-redis-node-1"
+        assert_equal "my-redis-node-1" [lindex [R 0 config get cluster-announce-human-nodename] 1]
+    }
+
+    # DoS prevention test: verify server can restart after CLUSTER SAVECONFIG
+    test "cluster-announce-ip persists correctly with CLUSTER SAVECONFIG" {
+        R 0 config set cluster-announce-ip "192.168.1.100"
+        R 0 cluster saveconfig
+
+        # Verify the IP appears in CLUSTER NODES output
+        assert_match "*192.168.1.100*" [R 0 cluster nodes]
+    }
+
+    test "cluster-announce-human-nodename persists correctly with CLUSTER SAVECONFIG" {
+        R 0 config set cluster-announce-human-nodename "production-node-1"
+        R 0 cluster saveconfig
+
+        # Verify the nodename is set correctly
+        assert_equal "production-node-1" [lindex [R 0 config get cluster-announce-human-nodename] 1]
+    }
 }