diff --git a/src/cluster.c b/src/cluster.c index 67d5149cb..2b03f331a 100644 --- a/src/cluster.c +++ b/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) { diff --git a/src/config.c b/src/config.c index e02cd64e5..8e08680fa 100644 --- a/src/config.c +++ b/src/config.c @@ -24,6 +24,7 @@ #include #include #include +#include /*----------------------------------------------------------------------------- * 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), diff --git a/tests/unit/cluster/announced-endpoints.tcl b/tests/unit/cluster/announced-endpoints.tcl index a37ca58d1..5784ea6f5 100644 --- a/tests/unit/cluster/announced-endpoints.tcl +++ b/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] + } }