Skip to content

MDEV-40151: Galera SST shell command injection via path parameters#5283

Open
hemantdangi-gc wants to merge 1 commit into
MariaDB:10.6from
mariadb-corporation:10.6-MDEV-40151
Open

MDEV-40151: Galera SST shell command injection via path parameters#5283
hemantdangi-gc wants to merge 1 commit into
MariaDB:10.6from
mariadb-corporation:10.6-MDEV-40151

Conversation

@hemantdangi-gc

Copy link
Copy Markdown
Contributor

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances security in Galera SST by validating directory and path parameters (such as datadir and innodb-data-home-dir) against shell-unsafe characters in both C++ and Bash before they are interpolated into shell commands. Feedback on these changes highlights a discrepancy between C++ and Bash validation where Bash's safe_path rejects UTF-8 characters allowed by C++, potentially breaking SST for non-ASCII paths. Additionally, potential buffer truncation issues were identified in snprintf formatting for ib_data_home_opt which could result in missing closing single quotes and command execution failures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +43 to +50
safe_path()
{
if [[ "${!1}" = *[!a-zA-Z0-9/\\._:\ +=,@%-]* ]]; then
wsrep_log_error "Invalid characters in $1: ${!1}"
exit 22
fi
echo "${!1}"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a discrepancy between the C++ validation (wsrep_path_char) and the Bash validation (safe_path).

In C++, wsrep_path_char explicitly allows all bytes >= 0x80 (which covers UTF-8 multi-byte characters) to support non-ASCII paths:

if (c >= 0x80) return true;

However, the Bash safe_path function uses a strict ASCII-only character class [!a-zA-Z0-9/\\._:\\ +=,@%-]. Any byte >= 0x80 will be matched by this negated class, causing safe_path to reject the path and abort the SST with exit code 22. This will break Galera SST for any database cluster where paths (such as datadir, innodb-data-home-dir, etc.) contain non-ASCII/UTF-8 characters.

To fix this, we should allow bytes in the range 0x80-0xFF in the Bash character class, and set LC_ALL=C locally within the function to ensure consistent byte-based range matching across different system locales.

Suggested change
safe_path()
{
if [[ "${!1}" = *[!a-zA-Z0-9/\\._:\ +=,@%-]* ]]; then
wsrep_log_error "Invalid characters in $1: ${!1}"
exit 22
fi
echo "${!1}"
}
safe_path()
{
local LC_ALL=C
if [[ "${!1}" = *[!a-zA-Z0-9/\\._:\\ +=,@%-$'\x80'-$'\xFF'-]* ]]; then
wsrep_log_error "Invalid characters in $1: ${!1}"
exit 22
fi
echo "${!1}"
}

Comment thread sql/wsrep_sst.cc
Comment on lines +1161 to +1165
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The stack buffer ib_data_home_opt has a fixed size of FN_REFLEN + 32. If data_home_dir is very long (e.g., close to FN_REFLEN or up to PATH_MAX on systems where paths can be longer than 512 characters), snprintf will truncate the formatted string.

Because the path is interpolated single-quoted ('%s'), truncation will result in a missing closing single quote ' at the end of the option. This will cause the shell to fail to parse the command, leading to a mysterious SST failure.

We should check the return value of snprintf to detect truncation and return an error if the path is too long to fit.

  char ib_data_home_opt[FN_REFLEN + 32];
  ib_data_home_opt[0]= '\0';
  if (data_home_dir)
  {
    int printed= snprintf(ib_data_home_opt, sizeof(ib_data_home_opt),
                          WSREP_SST_OPT_INNODB_DATA_HOME_DIR " '%s' ", data_home_dir);
    if (printed < 0 || (size_t)printed >= sizeof(ib_data_home_opt))
    {
      WSREP_ERROR("Refusing SST: innodb-data-home-dir path is too long.");
      return -EINVAL;
    }
  }

Comment thread sql/wsrep_sst.cc
Comment on lines +2006 to +2010
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the check in sst_prepare_other, we should check the return value of snprintf here to detect truncation of ib_data_home_opt and return an error if the path is too long to fit.

  char ib_data_home_opt[FN_REFLEN + 32];
  ib_data_home_opt[0]= '\0';
  if (data_home_dir)
  {
    int printed= snprintf(ib_data_home_opt, sizeof(ib_data_home_opt),
                          WSREP_SST_OPT_INNODB_DATA_HOME_DIR " '%s' ", data_home_dir);
    if (printed < 0 || (size_t)printed >= sizeof(ib_data_home_opt))
    {
      WSREP_ERROR("Refusing SST: innodb-data-home-dir path is too long.");
      return -EINVAL;
    }
  }

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant