Bug#37039394 Backport Bug# 36816986 - MySQL Shell command injection

Problem:
mysqldump does not sanitize the SQL statements
which might contain some special characters like
new line feed etc.

Because of this, a user can inject arbitrary shell
command(s) in to the dump file created by mysqldump.

And if that file is executed on a host machine, the
shell command will get executed on that host machine.

Solution:
The fix is to make sure that the rows which are being
fetched from dump_tablespaces are sanitized properly.

This fix is just sanitizing the rows coming in from
mysql_fetch_row while dumping tablespaces.

Change-Id: Ide74636ba60fd3cb6c936149f76b3c4ead70ecc9

Author: shussinh

client/mysqldump.c

@@ -54,6 +54,7 @@
 #include <m_string.h>
 #include <m_ctype.h>
 #include <hash.h>
+#include <ctype.h>
 #include <stdarg.h>
 
 #include "client_priv.h"
@@ -2328,6 +2329,82 @@ static char const* fix_identifier_with_newline(char const* object_name, my_bool*
 }
 
 
+/*
+ * fprintf_string:
+ * -- Print the escaped version of the given char* row into the md_result_file.
+ *
+ * @param[in] row                 the row to be printed
+ * @param[in] row_len             length of the row
+ * @param[in] quote               quote character, like ' or ` etc.
+ * @param[in] needs_newline       whether to print newline after the row
+ *
+ * @retval void
+ *
+ */
+static void fprintf_string(char *row, ulong row_len, char quote,
+                           my_bool needs_newline) {
+  // Create the buffer where we'll have sanitized row.
+  char buffer[2048];
+  char *pbuffer;
+  uint64_t curr_row_size;
+  pbuffer = &buffer[0];
+
+  curr_row_size = ((uint64_t)row_len) * 2 + 1;
+
+  // We'll allocate dynamic memory only for huge rows
+  if (curr_row_size > sizeof(buffer))
+    pbuffer = (char *)my_malloc(PSI_NOT_INSTRUMENTED, curr_row_size, MYF(0));
+
+  // Put the sanitized row in the buffer.
+  mysql_real_escape_string_quote(mysql, pbuffer, row, row_len, '\'');
+
+  // Opening quote
+  fputc(quote, md_result_file);
+
+  // Print the row to the file.
+  fputs(pbuffer, md_result_file);
+
+  // Closing quote
+  fputc(quote, md_result_file);
+
+  // Add the new line
+  if (needs_newline) fputc('\n', md_result_file);
+
+  // Free the buffer if we have to.
+  if (pbuffer != &buffer[0]) my_free(pbuffer);
+}
+
+/*
+ * is_string_integer:
+ * Check if the given string is a valid integer or not.
+ *
+ * @param[in]  str       number to be checked
+ * @param[in]  str_len   length of the string
+ *
+ * @retval     TRUE      if the string represents an integer
+ * @retval     FALSE     if the string has non-digit characters
+ */
+static my_bool is_string_integer(const char *str, ulong str_len) {
+  ulong start_index;
+  ulong i;
+
+  // Empty strings are invalid numbers
+  if (str_len == 0) return FALSE;
+
+  start_index = 0;
+
+  // For negative integers, start the index with 1
+  if (str[0] == '-') {
+    if (str_len == 1) return FALSE;
+    start_index = 1;
+  }
+
+  for (i = start_index; i < str_len; i++)
+    if (!isdigit(str[i])) return FALSE;
+
+  return TRUE;
+}
+
 /*
  create_delimiter
  Generate a new (null-terminated) string that does not exist in  query 
@@ -4440,6 +4517,7 @@ static int dump_tablespaces(char* ts_where)
   MYSQL_RES *tableres;
   char buf[FN_REFLEN];
   DYNAMIC_STRING sqlbuf;
+  ulong *lengths;
   int first= 0;
   /*
     The following are used for parsing the EXTRA field
@@ -4500,24 +4578,33 @@ static int dump_tablespaces(char* ts_where)
   buf[0]= 0;
   while ((row= mysql_fetch_row(tableres)))
   {
+    lengths = mysql_fetch_lengths(tableres);
     if (strcmp(buf, row[0]) != 0)
       first= 1;
     if (first)
     {
+       /*
+       * The print_comment below prints single line comments in the
+       * md_result_file (--). The single line comment is terminated by a new
+       * line, however because of the usage of mysql_real_escape_string_quote,
+       * the new line character will get escaped too in the string, hence
+       * another new line characters are being used at the end of the string
+       * to terminate the single line comment.
+       */
+      mysql_real_escape_string_quote(mysql, buf, row[0], lengths[0], '\'');
       print_comment(md_result_file, 0, "\n--\n-- Logfile group: %s\n--\n",
-                    row[0]);
-
+                    buf);
+      buf[0] = 0;
       fprintf(md_result_file, "\nCREATE");
     }
     else
     {
       fprintf(md_result_file, "\nALTER");
     }
-    fprintf(md_result_file,
-            " LOGFILE GROUP %s\n"
-            "  ADD UNDOFILE '%s'\n",
-            row[0],
-            row[1]);
+    fprintf(md_result_file, " LOGFILE GROUP ");
+    fprintf_string(row[0], lengths[0], '`', TRUE);
+    fprintf(md_result_file, "  ADD UNDOFILE ");
+    fprintf_string(row[1], lengths[1], '\'', TRUE);
     if (first)
     {
       ubs= strstr(row[5],extra_format);
@@ -4527,15 +4614,15 @@ static int dump_tablespaces(char* ts_where)
       endsemi= strstr(ubs,";");
       if(endsemi)
         endsemi[0]= '\0';
+      if (!is_string_integer(ubs, (ulong)strlen(ubs))) return 1;
       fprintf(md_result_file,
               "  UNDO_BUFFER_SIZE %s\n",
               ubs);
     }
-    fprintf(md_result_file,
-            "  INITIAL_SIZE %s\n"
-            "  ENGINE=%s;\n",
-            row[3],
-            row[4]);
+    if (!is_string_integer(row[3], lengths[3])) return 1;
+    fprintf(md_result_file, "  INITIAL_SIZE %s\n  ENGINE=", row[3]);
+    fprintf_string(row[4], lengths[4], '`', FALSE);
+    fprintf(md_result_file, ";\n");
     check_io(md_result_file);
     if (first)
     {
@@ -4567,38 +4654,54 @@ static int dump_tablespaces(char* ts_where)
     DBUG_RETURN(1);
   }
 
+  DBUG_EXECUTE_IF("tablespace_injection_test", {
+    mysql_free_result(tableres);
+    mysql_query_with_error_report(
+        mysql, &tableres,
+        "SELECT 'TN; /*' AS TABLESPACE_NAME, 'FN' AS FILE_NAME, 'LGN' AS "
+        "LOGFILE_GROUP_NAME, 77 AS EXTENT_SIZE, 88 AS INITIAL_SIZE, "
+        "'*/\nsystem touch foo;\n' AS ENGINE");
+  });
+
   buf[0]= 0;
   while ((row= mysql_fetch_row(tableres)))
   {
+    lengths = mysql_fetch_lengths(tableres);
     if (strcmp(buf, row[0]) != 0)
       first= 1;
     if (first)
     {
-      print_comment(md_result_file, 0, "\n--\n-- Tablespace: %s\n--\n", row[0]);
+      /*
+       * The print_comment below prints single line comments in the
+       * md_result_file (--). The single line comment is terminated by a new
+       * line, however because of the usage of mysql_real_escape_string_quote,
+       * the new line character will get escaped too in the string, hence
+       * another new line characters are being used at the end of the string
+       * to terminate the single line comment.
+       */
+      mysql_real_escape_string_quote(mysql, buf, row[0], lengths[0], '\'');
+      print_comment(md_result_file, 0, "\n--\n-- Tablespace: %s\n--\n", buf);
+      buf[0] = 0;
       fprintf(md_result_file, "\nCREATE");
     }
     else
     {
       fprintf(md_result_file, "\nALTER");
     }
-    fprintf(md_result_file,
-            " TABLESPACE %s\n"
-            "  ADD DATAFILE '%s'\n",
-            row[0],
-            row[1]);
-    if (first)
-    {
-      fprintf(md_result_file,
-              "  USE LOGFILE GROUP %s\n"
-              "  EXTENT_SIZE %s\n",
-              row[2],
-              row[3]);
+    fprintf(md_result_file, " TABLESPACE ");
+    fprintf_string(row[0], lengths[0], '`', TRUE);
+    fprintf(md_result_file, "  ADD DATAFILE ");
+    fprintf_string(row[1], lengths[1], '\'', TRUE);
+    if (first) {
+      fprintf(md_result_file, "  USE LOGFILE GROUP ");
+      fprintf_string(row[2], lengths[2], '`', TRUE);
+      if (!is_string_integer(row[3], lengths[3])) return 1;
+      fprintf(md_result_file, "  EXTENT_SIZE %s\n", row[3]);
     }
-    fprintf(md_result_file,
-            "  INITIAL_SIZE %s\n"
-            "  ENGINE=%s;\n",
-            row[4],
-            row[5]);
+    if (!is_string_integer(row[4], lengths[4])) return 1;
+    fprintf(md_result_file, "  INITIAL_SIZE %s\n  ENGINE=", row[4]);
+    fprintf_string(row[5], lengths[5], '`', FALSE);
+    fprintf(md_result_file, ";\n");
     check_io(md_result_file);
     if (first)
     {

mysql-test/r/mysqldump-tablespace-escape.result

@@ -0,0 +1,8 @@
+#
+# Bug#36816986 - MySQL Shell command injection
+#
+CREATE DATABASE bug36816986;
+-- Run mysqldump with tablespace_injection_test.
+The test injected string must be found:
+Pattern found.
+DROP DATABASE bug36816986;

mysql-test/t/mysqldump-tablespace-escape.test

@@ -0,0 +1,21 @@
+--source include/mysql_have_debug.inc
+
+--echo #
+--echo # Bug#36816986 - MySQL Shell command injection
+--echo #
+
+let $grep_file= $MYSQLTEST_VARDIR/tmp/bug36816986.sql;
+let $grep_output=boolean;
+
+CREATE DATABASE bug36816986;
+
+--echo -- Run mysqldump with tablespace_injection_test.
+--exec $MYSQL_DUMP --debug="d,tablespace_injection_test" --result-file=$grep_file bug36816986 --all-tablespaces 2>&1
+
+--echo The test injected string must be found:
+let $grep_pattern=qr|  ENGINE=\*/\nsystem touch foo|;
+--source include/grep_pattern.inc
+
+# Cleanup
+--remove_file $grep_file
+DROP DATABASE bug36816986;