From d5abe6d9fa8f4ed6b0549afd9544c6f72f4a1ab0 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Thu, 25 Jun 2026 20:22:39 +0530 Subject: [PATCH] MDEV-40151: Galera SST shell command injection via path parameters Issue: The datadir and InnoDB/Aria path parameters were interpolated single-quoted into shell commands run via sh -c (mysqld) and eval (rsync FILTER, mariabackup INNOEXTRA) without validation, so a single quote in a value broke out of the quoting and ran arbitrary commands as the mysql OS user. Solution: Add allowlist validators wsrep_path_char() (C++) and safe_path() (shell) that permit path characters incl. spaces but reject shell-breakout characters, and validate every such value before it reaches the command/eval. innodb-data-home-dir is now also passed on the SST command line (top priority) with env as fallback. --- .../include/galera_sst_param_injection.inc | 78 +++++++++++++++ .../r/galera_sst_buffer_pool_injection.result | 25 +++++ .../r/galera_sst_datadir_injection.result | 28 ++++++ .../r/galera_sst_dir_param_injection.result | 38 +++++++ .../t/galera_sst_buffer_pool_injection.cnf | 10 ++ .../t/galera_sst_buffer_pool_injection.test | 73 ++++++++++++++ .../galera/t/galera_sst_datadir_injection.cnf | 5 + .../t/galera_sst_datadir_injection.test | 98 +++++++++++++++++++ .../t/galera_sst_dir_param_injection.cnf | 5 + .../t/galera_sst_dir_param_injection.test | 56 +++++++++++ scripts/wsrep_sst_common.sh | 28 +++++- sql/wsrep_sst.cc | 93 +++++++++++++++++- sql/wsrep_sst.h | 1 + sql/wsrep_utils.cc | 13 +++ sql/wsrep_utils.h | 1 + 15 files changed, 547 insertions(+), 5 deletions(-) create mode 100644 mysql-test/suite/galera/include/galera_sst_param_injection.inc create mode 100644 mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result create mode 100644 mysql-test/suite/galera/r/galera_sst_datadir_injection.result create mode 100644 mysql-test/suite/galera/r/galera_sst_dir_param_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_injection.test create mode 100644 mysql-test/suite/galera/t/galera_sst_dir_param_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_dir_param_injection.test diff --git a/mysql-test/suite/galera/include/galera_sst_param_injection.inc b/mysql-test/suite/galera/include/galera_sst_param_injection.inc new file mode 100644 index 0000000000000..6449c002a4c8a --- /dev/null +++ b/mysql-test/suite/galera/include/galera_sst_param_injection.inc @@ -0,0 +1,78 @@ +# Drive one shell-injection rejection check for a single SST directory parameter +# that is validated by the SST script (safe_path). +# +# Caller sets before sourcing (and the cluster must be healthy, rsync SST): +# $inj_param - mysqld option name, clean text (e.g. innodb-log-group-home-dir) +# $inj_reject - log pattern in mysqld.2.err that proves the value was rejected +# +# Points $inj_param at a directory whose name contains a shell metacharacter and +# forces a full SST on node 2, so the restarted joiner runs the value through the +# SST script; asserts node 2 refuses it (logs $inj_reject). For +# innodb-data-home-dir the system tablespace (ibdata1) is moved into that dir so +# mysqld can still start. Restores the config and rejoins the cluster. + +--connection node_2 +--source include/shutdown_mysqld.inc + +# pass the (clean) option name to perl via a sidecar file +--exec echo "$inj_param" > $MYSQL_TMP_DIR/inj_param + +perl; + use strict; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + # read the option name from the sidecar (perl can't see mysqltest vars) + open(my $pf, '<', "$tmp/inj_param") or die $!; + my $param = <$pf>; chomp $param; close $pf; + my $ddir = "$tmp/pinj'x"; # name carries a shell metachar (') + my $cnf = "$vardir/my.cnf"; + mkpath($ddir); # must exist for mysqld to start + # innodb-data-home-dir holds the system tablespace: move ibdata1 in so the + # joiner can still start and reach the SST + rename("$vardir/mysqld.2/data/ibdata1", "$ddir/ibdata1") + if $param eq 'innodb-data-home-dir'; + # remember my.cnf size so cleanup can truncate the appended block away + open(my $sz, '>', "$tmp/inj_cnf_size") or die $!; print $sz -s $cnf; close $sz; + # point the option at the malicious dir (perl writes it, so no shell parses the ') + open(my $fh, '>>', $cnf) or die $!; + print $fh "[mysqld.2]\n$param=\"$ddir\"\n"; + close $fh; + unlink "$vardir/mysqld.2/data/grastate.dat"; # force a full SST +EOF + +# start the joiner; it must refuse SST and exit +--connection node_2 +--error 1,134 +--exec $MYSQLD_LAST_CMD + +# joiner must have rejected the value +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = $inj_reject +--source include/search_pattern_in_file.inc + +# cleanup: restore my.cnf, ibdata1 and the dir, rejoin. No stray helpers to kill +# - safe_path rejects in create_dirs, before the rsync daemon is ever started. +perl; + use strict; + use File::Path qw(rmtree); + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $cnf = "$vardir/my.cnf"; + my $ddir = "$tmp/pinj'x"; + open(my $pf, '<', "$tmp/inj_param") or die $!; + my $param = <$pf>; chomp $param; close $pf; + open(my $sz, '<', "$tmp/inj_cnf_size") or die $!; + my $orig = <$sz>; close $sz; + truncate($cnf, $orig) or die "truncate $cnf: $!"; + rename("$ddir/ibdata1", "$vardir/mysqld.2/data/ibdata1") + if $param eq 'innodb-data-home-dir'; + rmtree($ddir); + unlink "$tmp/inj_param", "$tmp/inj_cnf_size"; +EOF + +--connection node_2 +--source 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 diff --git a/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result b/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result new file mode 100644 index 0000000000000..21bf6cb7e96f4 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result @@ -0,0 +1,25 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +connection node_1; +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +FOUND 1 /Invalid characters in INNODB_BUFFER_POOL/ in mysqld.2.err +buffer-pool injection prevented +connection node_2; +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_mariabackup'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/r/galera_sst_datadir_injection.result b/mysql-test/suite/galera/r/galera_sst_datadir_injection.result new file mode 100644 index 0000000000000..00a3c04412d1e --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_datadir_injection.result @@ -0,0 +1,28 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +FOUND 1 /wsrep_sst_rsync/ in mysqld.1.err +connection node_1; +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +connection node_2; +FOUND 1 /unsafe for shell interpolation/ in mysqld.2.err +datadir injection prevented +connection node_2; +call mtr.add_suppression('Illegal character in variable'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/r/galera_sst_dir_param_injection.result b/mysql-test/suite/galera/r/galera_sst_dir_param_injection.result new file mode 100644 index 0000000000000..8ed66ee9e49f3 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_dir_param_injection.result @@ -0,0 +1,38 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +connection node_1; +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +connection node_2; +FOUND 1 /Invalid characters in/ in mysqld.2.err +connection node_2; +connection node_2; +connection node_2; +FOUND 2 /Invalid characters in/ in mysqld.2.err +connection node_2; +connection node_2; +connection node_2; +FOUND 3 /Invalid characters in/ in mysqld.2.err +connection node_2; +connection node_2; +connection node_2; +FOUND 4 /Invalid characters in/ in mysqld.2.err +connection node_2; +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf new file mode 100644 index 0000000000000..9013243cfb0c2 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf @@ -0,0 +1,10 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=mariabackup +wsrep_sst_auth="root:" +wsrep_debug=1 + +[sst] +transferfmt=@ENV.MTR_GALERA_TFMT +streamfmt=mbstream diff --git a/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test new file mode 100644 index 0000000000000..78edc1d69cb04 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test @@ -0,0 +1,73 @@ +# +# Galera mariabackup SST must reject innodb-buffer-pool-filename when its value +# contains shell metacharacters, instead of running it through the eval'd +# mariabackup command (INNOEXTRA). +# +# Steps: +# 1. Bring up a 2-node mariabackup-SST cluster. +# 2. Shut down node 2; force a full SST; restart it with a malicious +# innodb-buffer-pool-filename on the command line (so it is forwarded to +# the SST script via --mysqld-args). The value carries a "touch " +# payload. +# 3. Assert node 2 refuses the SST (safe_path) and the injected command never +# runs (no marker file). +# 4. Restart node 2 cleanly and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc +--source include/have_mariabackup.inc + +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 + +--connection node_1 +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +perl; + unlink "$ENV{MYSQL_TMP_DIR}/bp_inj_marker"; +EOF + +# restart the joiner with a malicious innodb-buffer-pool-filename on the command +# line; the payload is wrapped in double quotes so mtr's own shell does not run +# it (only the SST script's eval would, on an unfixed build) +--error 1,134 +--exec $MYSQLD_LAST_CMD --innodb-buffer-pool-filename="x'&touch $MYSQL_TMP_DIR/bp_inj_marker&'y" + +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = Invalid characters in INNODB_BUFFER_POOL +--source include/search_pattern_in_file.inc + +perl; + die "FAIL: marker created - buffer-pool injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/bp_inj_marker"; + print "buffer-pool injection prevented\n"; +EOF + +# cleanup: restart node 2 cleanly (no malicious arg) and rejoin +--connection node_2 +--source 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('Invalid characters in'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_mariabackup'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf b/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_injection.test b/mysql-test/suite/galera/t/galera_sst_datadir_injection.test new file mode 100644 index 0000000000000..c526dee1d1da3 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_injection.test @@ -0,0 +1,98 @@ +# +# Galera SST must reject a datadir containing shell metacharacters in mysqld +# (C++) before the SST command is built, instead of running it through the shell. +# +# Steps: +# 1. Bring up a 2-node rsync-SST cluster. +# 2. Shut down node 2; relocate its datadir to a name carrying a shell-injection +# payload (dd_inj'&touch &'x); force a full SST. +# 3. Start node 2 as joiner; assert it refuses the datadir and the injected +# command never runs (no marker file). +# 4. Restore the datadir and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +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 + +# sanity: initial SST used rsync +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = wsrep_sst_rsync +--source include/search_pattern_in_file.inc + +--connection node_1 +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +# move the datadir to a name carrying a shell-injection payload +perl; + use strict; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $marker = "$tmp/datadir_inj_marker"; + my $ddir = "$tmp/dd_inj'&touch $marker&'x"; + my $cnf = "$vardir/my.cnf"; + unlink $marker; + (my $parent = $ddir) =~ s{/[^/]+$}{}; + mkpath($parent); + rename("$vardir/mysqld.2/data", $ddir) or die "rename datadir: $!"; + unlink "$ddir/grastate.dat"; + open(my $sz, '>', "$tmp/inj_cnf_size") or die $!; print $sz -s $cnf; close $sz; + open(my $fh, '>>', $cnf) or die $!; print $fh "[mysqld.2]\ndatadir=\"$ddir\"\n"; close $fh; +EOF + +--connection node_2 +--error 1,134 +--exec $MYSQLD_LAST_CMD + +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = unsafe for shell interpolation +--source include/search_pattern_in_file.inc + +perl; + die "FAIL: marker created - datadir injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/datadir_inj_marker"; + print "datadir injection prevented\n"; +EOF + +# restore datadir and rejoin +perl; + use strict; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $cnf = "$vardir/my.cnf"; + open(my $sz, '<', "$tmp/inj_cnf_size") or die $!; my $orig = <$sz>; close $sz; + truncate($cnf, $orig) or die "truncate: $!"; + unlink "$tmp/datadir_inj_marker", "$tmp/inj_cnf_size"; + rename("$tmp/dd_inj'&touch $tmp/datadir_inj_marker&'x", "$vardir/mysqld.2/data") + or die "restore datadir: $!"; +EOF + +--connection node_2 +--source 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 + +# suppress the rejected-SST noise on the final node_2 instance +call mtr.add_suppression('Illegal character in variable'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_dir_param_injection.cnf b/mysql-test/suite/galera/t/galera_sst_dir_param_injection.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_dir_param_injection.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_dir_param_injection.test b/mysql-test/suite/galera/t/galera_sst_dir_param_injection.test new file mode 100644 index 0000000000000..7322117f886ff --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_dir_param_injection.test @@ -0,0 +1,56 @@ +# +# Galera SST must reject InnoDB/Aria directory parameters whose value contains +# shell metacharacters (the SST script's safe_path), as it does for datadir. +# +# Steps: +# 1. Bring up a 2-node rsync-SST cluster. +# 2. For each of innodb-data-home-dir, innodb-log-group-home-dir, +# innodb-undo-directory, aria-log-dir-path: point it at a directory whose +# name contains a shell metacharacter, force a full SST, and assert node 2 +# refuses it. +# 3. Restore config and rejoin after each parameter. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +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 + +--connection node_1 +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +# All four share galera_sst_param_injection.inc (it moves ibdata1 for +# innodb-data-home-dir so the joiner can still start). +--let $inj_reject = Invalid characters in + +--let $inj_param = innodb-data-home-dir +--source suite/galera/include/galera_sst_param_injection.inc + +--let $inj_param = innodb-log-group-home-dir +--source suite/galera/include/galera_sst_param_injection.inc + +--let $inj_param = innodb-undo-directory +--source suite/galera/include/galera_sst_param_injection.inc + +--let $inj_param = aria-log-dir-path +--source suite/galera/include/galera_sst_param_injection.inc + +# suppress the rejected-SST noise on the final node_2 instance +call mtr.add_suppression('Invalid characters in'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +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'); +call mtr.add_suppression('SST .* failed'); diff --git a/scripts/wsrep_sst_common.sh b/scripts/wsrep_sst_common.sh index 3009889cdefbd..64bd91c74f1a6 100644 --- a/scripts/wsrep_sst_common.sh +++ b/scripts/wsrep_sst_common.sh @@ -38,6 +38,17 @@ safe() echo "${!1}" } +# Like safe(), but for paths: allowlist of path chars (incl. space, unlike +# safe()); rejects anything that could break out of single-quoted shell/eval. +safe_path() +{ + if [[ "${!1}" = *[!a-zA-Z0-9/\\._:\ +=,@%-]* ]]; then + wsrep_log_error "Invalid characters in $1: ${!1}" + exit 22 + fi + echo "${!1}" +} + commandex() { if [ -n "$BASH_VERSION" ]; then @@ -753,8 +764,9 @@ if [ -z "$WSREP_SST_OPT_BINLOG" -a -n "${MYSQLD_OPT_LOG_BIN+x}" ]; then fi # Reconstructing the command line arguments that control the innodb -# and binlog options: +# and binlog options. if [ -n "$WSREP_SST_OPT_LOG_BASENAME" ]; then + safe_path WSREP_SST_OPT_LOG_BASENAME >/dev/null # validate (eval sink) if [ -n "$WSREP_SST_OPT_MYSQLD" ]; then WSREP_SST_OPT_MYSQLD="--log-basename='$WSREP_SST_OPT_LOG_BASENAME' $WSREP_SST_OPT_MYSQLD" else @@ -762,26 +774,34 @@ if [ -n "$WSREP_SST_OPT_LOG_BASENAME" ]; then fi fi if [ -n "$ARIA_LOG_DIR" ]; then + safe_path ARIA_LOG_DIR >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --aria-log-dir-path='$ARIA_LOG_DIR'" fi if [ -n "$INNODB_DATA_HOME_DIR" ]; then + safe_path INNODB_DATA_HOME_DIR >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --innodb-data-home-dir='$INNODB_DATA_HOME_DIR'" fi if [ -n "$INNODB_LOG_GROUP_HOME" ]; then + safe_path INNODB_LOG_GROUP_HOME >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --innodb-log-group-home-dir='$INNODB_LOG_GROUP_HOME'" fi if [ -n "$INNODB_UNDO_DIR" ]; then + safe_path INNODB_UNDO_DIR >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --innodb-undo-directory='$INNODB_UNDO_DIR'" fi if [ -n "$INNODB_BUFFER_POOL" ]; then + safe_path INNODB_BUFFER_POOL >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --innodb-buffer-pool-filename='$INNODB_BUFFER_POOL'" fi if [ -n "$INNODB_BUFFER_POOL_SIZE" ]; then + safe_path INNODB_BUFFER_POOL_SIZE >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --innodb-buffer-pool-size='$INNODB_BUFFER_POOL_SIZE'" fi if [ -n "$WSREP_SST_OPT_BINLOG" ]; then + safe_path WSREP_SST_OPT_BINLOG >/dev/null # validate (eval sink) INNOEXTRA="$INNOEXTRA --log-bin='$WSREP_SST_OPT_BINLOG'" if [ -n "$WSREP_SST_OPT_BINLOG_INDEX" ]; then + safe_path WSREP_SST_OPT_BINLOG_INDEX >/dev/null # validate (eval sink) if [ -n "$WSREP_SST_OPT_MYSQLD" ]; then WSREP_SST_OPT_MYSQLD="--log-bin-index='$WSREP_SST_OPT_BINLOG_INDEX' $WSREP_SST_OPT_MYSQLD" else @@ -1138,6 +1158,7 @@ wsrep_check_datadir() "The '--datadir' parameter must be passed to the SST script" exit 2 fi + safe_path WSREP_SST_OPT_DATA >/dev/null # validate ($DATA -> eval sinks) } get_openssl() @@ -1810,6 +1831,7 @@ create_data() DATA_DIR="$(pwd)" cd "$OLD_PWD" fi + safe_path DATA_DIR >/dev/null # validate (rsync FILTER / rsyncd.conf sink) } create_dirs() @@ -1832,6 +1854,7 @@ create_dirs() cd "$INNODB_DATA_HOME_DIR" ib_home_dir="$(pwd)" cd "$OLD_PWD" + safe_path ib_home_dir >/dev/null # validate (rsync FILTER / rsyncd.conf sink) [ $simplify -ne 0 -a "$ib_home_dir" = "$DATA_DIR" ] && ib_home_dir="" fi @@ -1851,6 +1874,7 @@ create_dirs() cd "$INNODB_LOG_GROUP_HOME" ib_log_dir="$(pwd)" cd "$OLD_PWD" + safe_path ib_log_dir >/dev/null # validate (rsync FILTER / rsyncd.conf sink) [ $simplify -ne 0 -a "$ib_log_dir" = "$DATA_DIR" ] && ib_log_dir="" fi @@ -1870,6 +1894,7 @@ create_dirs() cd "$INNODB_UNDO_DIR" ib_undo_dir="$(pwd)" cd "$OLD_PWD" + safe_path ib_undo_dir >/dev/null # validate (rsync FILTER / rsyncd.conf sink) [ $simplify -ne 0 -a "$ib_undo_dir" = "$DATA_DIR" ] && ib_undo_dir="" fi @@ -1888,6 +1913,7 @@ create_dirs() cd "$ARIA_LOG_DIR" ar_log_dir="$(pwd)" cd "$OLD_PWD" + safe_path ar_log_dir >/dev/null # validate (rsync FILTER / rsyncd.conf sink) [ $simplify -ne 0 -a "$ar_log_dir" = "$DATA_DIR" ] && ar_log_dir="" fi diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index 5fd81bec2c4cf..94a5d54ff5764 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -203,10 +203,24 @@ void wsrep_set_data_home_dir(const char *data_dir) data_home_dir= (data_dir && *data_dir) ? data_dir : NULL; } -static void make_wsrep_defaults_file() +/* Build the defaults-file options for the SST command. Validate them first + (they are interpolated single-quoted); return false if any is unsafe. */ +static bool make_wsrep_defaults_file() { if (!wsrep_defaults_file[0]) { + if ((my_defaults_file && + wsrep_check_request_str(my_defaults_file, wsrep_path_char, true)) || + (my_defaults_extra_file && + wsrep_check_request_str(my_defaults_extra_file, wsrep_path_char, true)) || + (my_defaults_group_suffix && + wsrep_check_request_str(my_defaults_group_suffix, wsrep_path_char, true))) + { + WSREP_ERROR("Refusing SST: a defaults-file option contains characters " + "that are unsafe for shell interpolation."); + return false; + } + char *ptr= wsrep_defaults_file; char *end= ptr + sizeof(wsrep_defaults_file); if (my_defaults_file) @@ -221,6 +235,7 @@ static void make_wsrep_defaults_file() ptr= strxnmov(ptr, end - ptr, WSREP_SST_OPT_CONF_SUFFIX, " '", my_defaults_group_suffix, "' ", NULL); } + return true; } @@ -1128,6 +1143,27 @@ static ssize_t sst_prepare_other (const char* method, return -ENOMEM; } + /* datadir and innodb-data-home-dir go single-quoted into the SST command; + reject shell-unsafe values (see wsrep_path_char). */ + if (wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true) || + (data_home_dir && + wsrep_check_request_str(data_home_dir, wsrep_path_char, true))) + { + WSREP_ERROR("Refusing SST: datadir or innodb-data-home-dir has characters " + "unsafe for shell interpolation."); + return -EINVAL; + } + + /* innodb-data-home-dir: command line (top priority) + env (fallback); + omitted when not configured. */ + char ib_data_home_opt[FN_REFLEN + 32]; + ib_data_home_opt[0]= '\0'; + if (data_home_dir) + { + snprintf(ib_data_home_opt, sizeof(ib_data_home_opt), + WSREP_SST_OPT_INNODB_DATA_HOME_DIR " '%s' ", data_home_dir); + } + char* binlog_opt_val= NULL; char* binlog_index_opt_val= NULL; @@ -1147,7 +1183,12 @@ static ssize_t sst_prepare_other (const char* method, return ret; } - make_wsrep_defaults_file(); + if (!make_wsrep_defaults_file()) + { + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } ret= snprintf (cmd_str(), cmd_len, "wsrep_sst_%s " @@ -1155,11 +1196,13 @@ static ssize_t sst_prepare_other (const char* method, WSREP_SST_OPT_ADDR " '%s' " WSREP_SST_OPT_DATA " '%s' " "%s" + "%s" WSREP_SST_OPT_PARENT " %d " WSREP_SST_OPT_PROGRESS " %d" "%s" "%s", method, addr_in, mysql_real_data_home, + ib_data_home_opt, wsrep_defaults_file, (int)getpid(), 0, @@ -1528,6 +1571,17 @@ static int sst_donate_mysqldump (const char* addr, return -ENOMEM; } + /* socket path and datadir go single-quoted into the SST command; reject + shell-unsafe values (see wsrep_path_char). */ + if ((mysqld_unix_port && + wsrep_check_request_str(mysqld_unix_port, wsrep_path_char, true)) || + wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true)) + { + WSREP_ERROR("Refusing SST: socket or datadir has characters unsafe for " + "shell interpolation."); + return -EINVAL; + } + /* we enable new client connections so that mysqldump donation can connect in, but we reject local connections from modifyingcdata during SST, to keep @@ -1535,7 +1589,8 @@ static int sst_donate_mysqldump (const char* addr, */ if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(TRUE); - make_wsrep_defaults_file(); + if (!make_wsrep_defaults_file()) + return -EINVAL; std::ostringstream uuid_oss; uuid_oss << gtid.id(); @@ -1933,6 +1988,29 @@ static int sst_donate_other (const char* method, return -ENOMEM; } + /* socket path, datadir and innodb-data-home-dir go single-quoted into the SST + command; reject shell-unsafe values (see wsrep_path_char). */ + if ((mysqld_unix_port && + wsrep_check_request_str(mysqld_unix_port, wsrep_path_char, true)) || + wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true) || + (data_home_dir && + wsrep_check_request_str(data_home_dir, wsrep_path_char, true))) + { + WSREP_ERROR("Refusing SST: socket, datadir or innodb-data-home-dir has " + "characters unsafe for shell interpolation."); + return -EINVAL; + } + + /* innodb-data-home-dir: command line (top priority) + env (fallback); + omitted when not configured. */ + char ib_data_home_opt[FN_REFLEN + 32]; + ib_data_home_opt[0]= '\0'; + if (data_home_dir) + { + snprintf(ib_data_home_opt, sizeof(ib_data_home_opt), + WSREP_SST_OPT_INNODB_DATA_HOME_DIR " '%s' ", data_home_dir); + } + char* binlog_opt_val= NULL; char* binlog_index_opt_val= NULL; @@ -1951,7 +2029,12 @@ static int sst_donate_other (const char* method, return ret; } - make_wsrep_defaults_file(); + if (!make_wsrep_defaults_file()) + { + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } std::ostringstream uuid_oss; uuid_oss << gtid.id(); @@ -1964,6 +2047,7 @@ static int sst_donate_other (const char* method, WSREP_SST_OPT_PROGRESS " %d " WSREP_SST_OPT_DATA " '%s' " "%s" + "%s" WSREP_SST_OPT_GTID " '%s:%lld' " WSREP_SST_OPT_GTID_DOMAIN_ID " %d" "%s" @@ -1972,6 +2056,7 @@ static int sst_donate_other (const char* method, method, addr, mysqld_port, mysqld_unix_port, 0, mysql_real_data_home, + ib_data_home_opt, wsrep_defaults_file, uuid_oss.str().c_str(), gtid.seqno().get(), wsrep_gtid_server.domain_id, binlog_opt_val, binlog_index_opt_val, diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index 462db7a159edd..39c4c6a4e0102 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -26,6 +26,7 @@ #define WSREP_SST_OPT_ADDR "--address" #define WSREP_SST_OPT_AUTH "--auth" #define WSREP_SST_OPT_DATA "--datadir" +#define WSREP_SST_OPT_INNODB_DATA_HOME_DIR "--innodb-data-home-dir" #define WSREP_SST_OPT_CONF "--defaults-file" #define WSREP_SST_OPT_CONF_SUFFIX "--defaults-group-suffix" #define WSREP_SST_OPT_CONF_EXTRA "--defaults-extra-file" diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index 1cb4fcfb562ae..dae726daea2c9 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -619,6 +619,19 @@ bool wsrep_address_char(const unsigned char c) (c == ':') || (c == '[') || (c == ']') || (c == '/'); } +/* Path char that is also safe single-quoted in shell/eval: allowlist of path + chars (incl. space) + bytes >= 0x80 (UTF-8); rejects all shell metacharacters + (notably ') so a validated path cannot break out of its quoting. */ +bool wsrep_path_char(const unsigned char c) +{ + if (c >= 0x80) return true; /* UTF-8: never a metacharacter */ + return wsrep_filename_char(c) || /* alnum . - _ */ + (c == '/') || (c == '\\') || /* separators */ + (c == ':') || (c == ' ') || /* drive letter; space */ + (c == '+') || (c == '=') || (c == ',') || (c == '@') || (c == '%'); + /* punctuation valid in filenames */ +} + bool wsrep_shell_char(const unsigned char c) { return (c != '`') && (c != '\'') && (c != '$') && diff --git a/sql/wsrep_utils.h b/sql/wsrep_utils.h index 9d66519c7deab..1bb48e14348e8 100644 --- a/sql/wsrep_utils.h +++ b/sql/wsrep_utils.h @@ -428,6 +428,7 @@ class critical bool wsrep_filename_char(const unsigned char c); bool wsrep_comma_char(const unsigned char c); bool wsrep_address_char(const unsigned char c); +bool wsrep_path_char(const unsigned char c); bool wsrep_shell_char(const unsigned char c); bool wsrep_names_list(const unsigned char c); bool wsrep_check_request_str(const char* const str,