Bug#36248967: Issue in mysqldump (mysql dump utility)

Problem: mysqldump not sanitizing the version string obtained from
server which may lead to injecting malicious commands to the output.

Fix: added function sanitizing the version string by cutting off
illegal part and issuing warning.

Test: check the server version in the output with and without injected
payload.

Change-Id: I1f19e1c90bdb8d444285e427092face3bb16da01

Author: Michal Jankowski

client/mysqldump.cc

@@ -759,6 +759,36 @@ static void short_usage(void) {
   printf("For more options, use %s --help\n", my_progname);
 }
 
+static void get_safe_server_info(char *safe_server_info,
+                                 size_t safe_server_info_len) {
+  const char *server_info = mysql_get_server_info(&mysql_connection);
+  if (server_info == nullptr) {
+    safe_server_info[0] = 0;
+    return;
+  }
+  DBUG_EXECUTE_IF("server_version_injection_test", {
+    const char *payload = "8.0.0-injection_test\n\\! touch /tmp/xxx";
+    server_info = payload;
+  });
+  for (size_t i = 0; i < safe_server_info_len; ++i) {
+    // End of string.
+    if (server_info[i] == 0) {
+      safe_server_info[i] = 0;
+      return;
+    }
+    // Version may include only alphanumeric and punctuation characters.
+    // Cut off the rest of the string if incorrect character found.
+    if (!(isalnum(server_info[i]) || ispunct(server_info[i]))) {
+      safe_server_info[i] = 0;
+      fprintf(stderr,
+              "-- Warning: version string returned by server is incorrect.\n");
+      return;
+    }
+    safe_server_info[i] = server_info[i];
+  }
+  safe_server_info[safe_server_info_len - 1] = 0;
+}
+
 static void write_header(FILE *sql_file, char *db_name) {
   if (opt_xml) {
     fputs("<?xml version=\"1.0\"?>\n", sql_file);
@@ -777,15 +807,16 @@ static void write_header(FILE *sql_file, char *db_name) {
 
     bool freemem = false;
     char const *text = fix_identifier_with_newline(db_name, &freemem);
+    char safe_server_info[SERVER_VERSION_LENGTH];
+    get_safe_server_info(safe_server_info, SERVER_VERSION_LENGTH);
     print_comment(sql_file, false, "-- Host: %s    Database: %s\n",
                   current_host ? current_host : "localhost", text);
     if (freemem) my_free(const_cast<char *>(text));
 
     print_comment(
         sql_file, false,
         "-- ------------------------------------------------------\n");
-    print_comment(sql_file, false, "-- Server version\t%s\n",
-                  mysql_get_server_info(&mysql_connection));
+    print_comment(sql_file, false, "-- Server version\t%s\n", safe_server_info);
 
     if (opt_set_charset)
       fprintf(

mysql-test/r/mysqldump_debug_bugs.result

@@ -0,0 +1,14 @@
+#
+# Bug#36248967: Security issue in mysqldump (mysql dump utility)
+#
+CREATE DATABASE test_bug36248967;
+-- Run mysqldump with payload injected to server version.
+A warning must be issued.
+-- Warning: version string returned by server is incorrect.
+The test version must be found:
+Pattern found.
+The test injected string must not be found:
+Pattern not found.
+-- Run mysqldump with correct server version.
+A warning must not be issued.
+DROP DATABASE test_bug36248967;

mysql-test/t/mysqldump_debug_bugs.test

@@ -0,0 +1,30 @@
+--source include/have_debug.inc
+
+--echo #
+--echo # Bug#36248967: Security issue in mysqldump (mysql dump utility)
+--echo #
+
+let $grep_file=$MYSQLTEST_VARDIR/tmp/bug36248967.sql;
+let $grep_output=boolean;
+
+CREATE DATABASE test_bug36248967;
+
+--echo -- Run mysqldump with payload injected to server version.
+--echo A warning must be issued.
+--exec $MYSQL_DUMP --debug="d,server_version_injection_test" --result-file=$grep_file test_bug36248967 2>&1
+
+--echo The test version must be found:
+let $grep_pattern=8.0.0-injection_test;
+--source include/grep_pattern.inc
+
+--echo The test injected string must not be found:
+let $grep_pattern=\\! touch /tmp/xxx;
+--source include/grep_pattern.inc
+
+--echo -- Run mysqldump with correct server version.
+--exec $MYSQL_DUMP --result-file=$grep_file test_bug36248967 2>&1
+--echo A warning must not be issued.
+
+#Cleanup
+--remove_file $grep_file
+DROP DATABASE test_bug36248967;