diff --git a/mysql-test/suite/galera/r/galera_sst_cn_injection.result b/mysql-test/suite/galera/r/galera_sst_cn_injection.result new file mode 100644 index 0000000000000..9b177d388657b --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_cn_injection.result @@ -0,0 +1,24 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +# +# MDEV-39413 wsrep unsafe handling of parameters +# +call mtr.add_suppression("WSREP: Illegal character in state transfer request:"); +call mtr.add_suppression("WSREP: Bad remote auth string. SST canceled."); +call mtr.add_suppression("WSREP: SST preparation failed"); +call mtr.add_suppression("WSREP: SST failed:"); +call mtr.add_suppression(".*State transfer to:*"); +call mtr.add_suppression("WSREP: Illegal character in variable:"); +connection node_2; +# force SST again +# using a cert with shell-unsafe CN +# start the server +# the server failed to start +FOUND 1 /Bad remote auth string. SST canceled./ in mysqld.1.err +# cleanup +# restart +call mtr.add_suppression(".*WSREP.*Will never receive state. Need to abort."); +call mtr.add_suppression("WSREP_SST:"); diff --git a/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result b/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result deleted file mode 100644 index 48b296a96f56d..0000000000000 --- a/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result +++ /dev/null @@ -1,27 +0,0 @@ -connection node_2; -connection node_1; -SELECT 1; -1 -1 -connection node_2; -FOUND 1 /wsrep_sst_rsync/ in mysqld.1.err -connection node_1; -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_rsync'); -call mtr.add_suppression('Process completed with error: wsrep_sst_rsync'); -call mtr.add_suppression('Command did not run: wsrep_sst_rsync'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/sst_in_progress'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -connection node_2; -connection node_1; -FOUND 1 /Invalid value for WSREP_SST_OPT_REMOTE_USER/ in mysqld.1.err -connection node_2; -# restart -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('State transfer to .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf b/mysql-test/suite/galera/t/galera_sst_cn_injection.cnf similarity index 91% rename from mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf rename to mysql-test/suite/galera/t/galera_sst_cn_injection.cnf index 9b919145a316d..9ec227ec3db66 100644 --- a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.cnf @@ -1,7 +1,6 @@ !include ../galera_2nodes.cnf [mysqld] -wsrep_sst_method=rsync wsrep_sst_auth="root:" wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations b/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations new file mode 100644 index 0000000000000..1f8f972f5cbe2 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations @@ -0,0 +1,6 @@ +[rsync] +wsrep-sst-method=rsync + +[mariabackup] +wsrep-sst-method=mariabackup + diff --git a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test b/mysql-test/suite/galera/t/galera_sst_cn_injection.test similarity index 68% rename from mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test rename to mysql-test/suite/galera/t/galera_sst_cn_injection.test index 0418be4d797a0..8458e6fe4b3b5 100644 --- a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.test @@ -15,24 +15,16 @@ SELECT 1; --let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; --source include/wait_condition.inc -# Confirm that transfer was SSL-encrypted ---let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err ---let SEARCH_PATTERN = Using openssl based encryption with socat: with key and crt ---source include/search_pattern_in_file.inc - --echo # --echo # MDEV-39413 wsrep unsafe handling of parameters --echo # -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_mariabackup'); -call mtr.add_suppression('Process completed with error: wsrep_sst_mariabackup'); -call mtr.add_suppression('Command did not run: wsrep_sst_mariabackup'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal'); +call mtr.add_suppression("WSREP: Illegal character in state transfer request:"); +call mtr.add_suppression("WSREP: Bad remote auth string. SST canceled."); +call mtr.add_suppression("WSREP: SST preparation failed"); +call mtr.add_suppression("WSREP: SST failed:"); +call mtr.add_suppression(".*State transfer to:*"); +call mtr.add_suppression("WSREP: Illegal character in variable:"); --connection node_2 --source include/shutdown_mysqld.inc @@ -51,7 +43,8 @@ call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal' --exec $MYSQLD_LAST_CMD --echo # the server failed to start ---let SEARCH_PATTERN = Invalid value for WSREP_SST_OPT_REMOTE_USER +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = Bad remote auth string. SST canceled. --source include/search_pattern_in_file.inc --echo # cleanup @@ -60,6 +53,10 @@ call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal' # pkill exit code varies by platform 0 or 1 --error 0,1 --exec pkill -f 'socat.*server-new-cert' + --exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-cert.pem >> $MYSQLTEST_VARDIR/my.cnf --exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-key.pem >> $MYSQLTEST_VARDIR/my.cnf --source $MYSQL_TEST_DIR/include/start_mysqld.inc + +call mtr.add_suppression(".*WSREP.*Will never receive state. Need to abort."); +call mtr.add_suppression("WSREP_SST:"); diff --git a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf b/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf deleted file mode 100644 index 0dc79df5a8086..0000000000000 --- a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf +++ /dev/null @@ -1,13 +0,0 @@ -!include ../galera_2nodes.cnf - -[mysqld] -wsrep_sst_method=mariabackup -wsrep_sst_auth="root:" -wsrep_debug=1 - -ssl-cert=@ENV.MYSQL_TEST_DIR/std_data/server-cert.pem -ssl-key=@ENV.MYSQL_TEST_DIR/std_data/server-key.pem -ssl-ca=@ENV.MYSQL_TEST_DIR/std_data/cacert.pem - -[sst] -ssl-mode=VERIFY_CA diff --git a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test b/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test deleted file mode 100644 index f0458e1f62fd2..0000000000000 --- a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test +++ /dev/null @@ -1,99 +0,0 @@ -# -# Verifies that wsrep_sst_rsync.sh rejects a joiner-supplied certificate -# whose CN contains shell-unsafe characters. -# -# Brings up a 2-node cluster with rsync SST and ssl-mode=VERIFY_CA, then -# forces a fresh SST on node_2 using std_data/server-new-cert.pem -- a -# cert whose CN intentionally contains shell metacharacters. Confirms -# that the donor (node_1) logs -# "Invalid value for WSREP_SST_OPT_REMOTE_USER" -# i.e. the rsync SST script refuses the value rather than interpolating -# it into stunnel.conf or the rsync magic file. -# - ---source include/galera_cluster.inc ---source include/have_innodb.inc - -SELECT 1; - ---connection node_2 ---let $wait_condition = SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'WSREP_LOCAL_STATE_COMMENT' ---source include/wait_condition.inc - -# Confirm the initial SST went via rsync + stunnel (sanity check for the -# test configuration). ---let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err ---let SEARCH_PATTERN = wsrep_sst_rsync ---source include/search_pattern_in_file.inc - - -# Reject shell-unsafe joiner-supplied auth (rsync) - -# Suppressions are per-server. node_1 will log the donor-side rejection -# ("Invalid value for WSREP_SST_OPT_REMOTE_USER"); node_2 will log the -# joiner-side "Will never receive state" abort. Add to both. ---connection node_1 -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_rsync'); -call mtr.add_suppression('Process completed with error: wsrep_sst_rsync'); -call mtr.add_suppression('Command did not run: wsrep_sst_rsync'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/sst_in_progress'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); - ---connection node_2 ---source include/shutdown_mysqld.inc - -# force SST again ---remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat -# using a cert with shell-unsafe CN ---exec echo '[mysqld.2]' >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-new-cert.pem >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-new-key.pem >> $MYSQLTEST_VARDIR/my.cnf - -# start the server -# Joiner mariadbd exits when SST is aborted; the exit code varies by -# platform (clean 0 on some systems, signalled 134 / 1 on others). ---error 0,1,134 ---exec $MYSQLD_LAST_CMD -# the donor refused the SST request - ---connection node_1 -# safe() in wsrep_sst_common.sh logs this when it rejects the joiner CN; -# wsrep_sst_rsync.sh wraps the joiner-supplied REMOTE_USER with $(safe ..) -# at line 249 so the value never reaches the stunnel.conf heredoc. ---let SEARCH_PATTERN = Invalid value for WSREP_SST_OPT_REMOTE_USER ---source include/search_pattern_in_file.inc - -# cleanup -# Kill joiner's stunnel / rsync that may linger after the aborted SST. -# Use a perl block because --exec with pkill -f matches the mtr cmdline -# itself (which contains the pattern) and tears down the wrong process. -perl; - open(my $fh, '-|', 'ps', '-eo', 'pid,args') or die "ps: $!"; - while (<$fh>) { - next unless /server-new-cert/; - next unless /^\s*(\d+)\s+(?:.*\/)?(stunnel|socat|rsync)\b/; - kill 'TERM', $1; - } - close $fh; -EOF ---exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-cert.pem >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-key.pem >> $MYSQLTEST_VARDIR/my.cnf - -# Switch back to node_2 before restarting it; the connection associates -# with the soon-to-be-restarted server so mtr auto-reconnects and the -# wait_condition + late suppressions land on the new instance. ---connection node_2 ---source $MYSQL_TEST_DIR/include/start_mysqld.inc - ---let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' ---source include/wait_condition.inc - -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('State transfer to .* failed'); diff --git a/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result b/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result index e9a93d2fcd617..3c011674b61e6 100644 --- a/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result +++ b/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result @@ -31,6 +31,26 @@ ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of 'NULL' SELECT @@global.wsrep_node_address; @@global.wsrep_node_address +SET @@global.wsrep_node_address="127.0.0.1'ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1'ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1 ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1 ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1`ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1`ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1$ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1$ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + SET @@global.wsrep_node_address=ON; SELECT @@global.wsrep_node_address; @@global.wsrep_node_address diff --git a/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result b/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result index e6b532c6bba25..4ffd6df81e8f1 100644 --- a/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result +++ b/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result @@ -46,7 +46,29 @@ SELECT @@global.wsrep_sst_auth; NULL SET @@global.wsrep_sst_auth=user:pass; ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ':pass' at line 1 +SET @@global.wsrep_sst_auth='user new:pass'; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user new:pass' +SET @@global.wsrep_sst_auth='user$new:pass'; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user$new:pass' +SET @@global.wsrep_sst_auth="user'new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user'new:pass' +SET @@global.wsrep_sst_auth="user;new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user;new:pass' +SET @@global.wsrep_sst_auth="user|new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user|new:pass' +SET @@global.wsrep_sst_auth="user&new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user&new:pass' +SET @@global.wsrep_sst_auth="user\\new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user\new:pass' +SET @@global.wsrep_sst_auth="user\"new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user"new:pass' +SET @@global.wsrep_sst_auth="user\nnew:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user +new:pass' +SET @@global.wsrep_sst_auth="user\tnew:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user new:pass' # restore the initial value SET @@global.wsrep_sst_auth = @wsrep_sst_auth_global_saved; +call mtr.add_suppression("WSREP: Illegal character in variable:"); # End of test diff --git a/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test b/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test index fccb40de6bf7c..400aa90289781 100644 --- a/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test +++ b/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test @@ -29,6 +29,19 @@ SELECT @@global.wsrep_node_address; --error ER_WRONG_VALUE_FOR_VAR SET @@global.wsrep_node_address=NULL; SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1'ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1 ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1`ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1$ps"; +SELECT @@global.wsrep_node_address; + # The values being assigned to wsrep_node_address are not verified so the # following alues are currently valid too. SET @@global.wsrep_node_address=ON; diff --git a/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test b/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test index aa901ef9ff792..1961bc89652ff 100644 --- a/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test +++ b/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test @@ -37,9 +37,31 @@ SET @@global.wsrep_sst_auth=1; SELECT @@global.wsrep_sst_auth; --error ER_PARSE_ERROR SET @@global.wsrep_sst_auth=user:pass; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth='user new:pass'; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth='user$new:pass'; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user'new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user;new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user|new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user&new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\\new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\"new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\nnew:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\tnew:pass"; --echo --echo # restore the initial value SET @@global.wsrep_sst_auth = @wsrep_sst_auth_global_saved; +call mtr.add_suppression("WSREP: Illegal character in variable:"); + --echo # End of test diff --git a/scripts/wsrep_sst_mariabackup.sh b/scripts/wsrep_sst_mariabackup.sh index 1ced7087f7033..e3e52d92fcbc3 100644 --- a/scripts/wsrep_sst_mariabackup.sh +++ b/scripts/wsrep_sst_mariabackup.sh @@ -1117,7 +1117,8 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then wsrep_log_info "Using '$itmpdir' as mariadb-backup working directory" if [ -n "$WSREP_SST_OPT_USER" ]; then - INNOEXTRA="$INNOEXTRA --user='$WSREP_SST_OPT_USER'" + WSREP_SST_OPT_USER_SAFE="$(safe WSREP_SST_OPT_USER)" + INNOEXTRA="$INNOEXTRA --user='$WSREP_SST_OPT_USER_SAFE'" fi if [ -n "$WSREP_SST_OPT_PSWD" ]; then diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index d40c7a77026c4..5fd81bec2c4cf 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -172,48 +172,6 @@ static void* wsrep_sst_joiner_monitor_thread(void *arg __attribute__((unused))) return NULL; } -/* return true if character can be a part of a filename */ -static bool filename_char(int const c) -{ - return isalnum(c) || (c == '-') || (c == '_') || (c == '.'); -} - -/* return true if string is comma seprated list */ -static bool comma_char(int const c) -{ - return (c == ','); -} - -/* return true if character can be a part of an address string */ -static bool address_char(int const c) -{ - return filename_char(c) || - (c == ':') || (c == '[') || (c == ']') || (c == '/'); -} - -/* return true if character can be a part of an address string list */ -static bool names_list(int const c) -{ - return address_char(c) || comma_char(c); -} - -static bool check_request_str(const char* const str, - bool (*check) (int c), - bool log_warn = true) -{ - for (size_t i(0); str[i] != '\0'; ++i) - { - if (!check(str[i])) - { - if (log_warn) WSREP_WARN("Illegal character in state transfer request: %i (%c).", - str[i], str[i]); - return true; - } - } - - return false; -} - bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var) { if ((! var->save_result.string_value.str) || @@ -226,8 +184,8 @@ bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var) } /* check also that method name is alphanumeric string */ - if (check_request_str(var->save_result.string_value.str, - filename_char, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_filename_char, false)) { my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, var->save_result.string_value.str ? @@ -279,8 +237,8 @@ bool wsrep_sst_receive_address_check (sys_var *self, THD* thd, set_var* var) } /* check also that address contains only accepted characters */ - if (check_request_str(var->save_result.string_value.str, - address_char, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_address_char, false)) { goto err; } @@ -302,7 +260,37 @@ bool wsrep_sst_receive_address_update (sys_var *self, THD* thd, bool wsrep_sst_auth_check (sys_var *self, THD* thd, set_var* var) { + /* Allow empty value */ + if (!var->save_result.string_value.str || var->save_result.string_value.length == 0) return 0; + + /* Check length */ + if ((var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + { + goto err; + } + + { + /* Split sst_auth on ':'-character */ + std::string auth= var->save_result.string_value.str; + std::string r_user= auth.substr(0, auth.find(":")); + + /* check also that user contains only accepted characters, + password part is not validated. */ + if (wsrep_check_request_str(r_user.c_str(), + wsrep_filename_char, true)) + { + goto err; + } + } + + return 0; + +err: + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); + return 1; } static bool sst_auth_real_set (const char* value) @@ -372,8 +360,8 @@ bool wsrep_sst_donor_check (sys_var *self, THD* thd, set_var* var) return 0; /* check also that donor string contains only accepted characters */ - if (check_request_str(var->save_result.string_value.str, - names_list, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_names_list, false)) { goto err; } @@ -2030,7 +2018,7 @@ int wsrep_sst_donate(const std::string& msg, const char* method= msg.data(); size_t method_len= strlen (method); - if (check_request_str(method, filename_char, true)) + if (wsrep_check_request_str(method, wsrep_filename_char, true)) { WSREP_ERROR("Bad SST method name. SST canceled."); return WSREP_CB_FAILURE; @@ -2052,7 +2040,14 @@ int wsrep_sst_donate(const std::string& msg, addr= data; } - if (check_request_str(addr, address_char, true)) + if (remote_auth() && + wsrep_check_request_str(remote_auth(), wsrep_shell_char, true)) + { + WSREP_ERROR("Bad remote auth string. SST canceled."); + return WSREP_CB_FAILURE; + } + + if (wsrep_check_request_str(addr, wsrep_address_char, true)) { WSREP_ERROR("Bad SST address string. SST canceled."); return WSREP_CB_FAILURE; diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index a8098b560e683..5cda63e52fb39 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -1,4 +1,5 @@ /* Copyright 2010-2015 Codership Oy + Copyright 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -598,3 +599,50 @@ size_t wsrep_host_len(const char* const addr, size_t const addr_len) return (colon ? colon - addr : addr_len); } } + +/* return true if character can be a part of a filename */ +bool wsrep_filename_char(int const c) +{ + return isalnum(c) || (c == '-') || (c == '_') || (c == '.'); +} + +/* return true if string is comma seprated list */ +bool wsrep_comma_char(int const c) +{ + return (c == ','); +} + +/* return true if character can be a part of an address string */ +bool wsrep_address_char(int const c) +{ + return wsrep_filename_char(c) || + (c == ':') || (c == '[') || (c == ']') || (c == '/'); +} + +bool wsrep_shell_char(int const c) +{ + return (c != '`') && (c != '\'') && (c != '$') && (c != ' '); +} + +/* return true if character can be a part of an address string list */ +bool wsrep_names_list(int const c) +{ + return wsrep_address_char(c) || wsrep_comma_char(c); +} + +bool wsrep_check_request_str(const char* const str, + bool (*check) (int c), + bool log_warn) +{ + for (size_t i(0); str[i] != '\0'; ++i) + { + if (!check(str[i])) + { + if (log_warn) WSREP_WARN("Illegal character in variable: %i (%c).", + str[i], str[i]); + return true; + } + } + + return false; +} diff --git a/sql/wsrep_utils.h b/sql/wsrep_utils.h index d34b15e3b09bb..648ed50672d5a 100644 --- a/sql/wsrep_utils.h +++ b/sql/wsrep_utils.h @@ -1,4 +1,5 @@ /* Copyright (C) 2013-2015 Codership Oy + Copyright (C) 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -423,4 +424,14 @@ class critical } // namespace wsrep +// Functions to validate dynamic variable safety +bool wsrep_filename_char(int const c); +bool wsrep_comma_char(int const c); +bool wsrep_address_char(int const c); +bool wsrep_shell_char(int const c); +bool wsrep_names_list(int const c); +bool wsrep_check_request_str(const char* const str, + bool (*check) (int c), + bool log_warn = true); + #endif /* WSREP_UTILS_H */ diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc index 152563a061d57..306a07151ca51 100644 --- a/sql/wsrep_var.cc +++ b/sql/wsrep_var.cc @@ -1,4 +1,5 @@ /* Copyright 2008-2023 Codership Oy + Copyright 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -632,10 +633,10 @@ static int wsrep_cluster_address_verify (const char* cluster_address_str) bool wsrep_cluster_address_check (sys_var *self, THD* thd, set_var* var) { - char addr_buf[FN_REFLEN]; + char addr_buf[FN_REFLEN+1]; if ((! var->save_result.string_value.str) || - (var->save_result.string_value.length >= sizeof(addr_buf))) // safety + (var->save_result.string_value.length >= FN_REFLEN)) // safety goto err; strmake(addr_buf, var->save_result.string_value.str, @@ -738,20 +739,19 @@ bool wsrep_node_name_update (sys_var *self, THD* thd, enum_var_type type) return 0; } -// TODO: do something more elaborate, like checking connectivity bool wsrep_node_address_check (sys_var *self, THD* thd, set_var* var) { - char addr_buf[FN_REFLEN]; - if ((! var->save_result.string_value.str) || - (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + (var->save_result.string_value.length >= FN_REFLEN)) // safety goto err; - memcpy(addr_buf, var->save_result.string_value.str, - var->save_result.string_value.length); - addr_buf[var->save_result.string_value.length]= 0; + if (var->save_result.string_value.length) + { + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_address_char, false)) + goto err; + } - // TODO: for now 'allow' 0-length string to be valid (default) return 0; err: