Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-4664

mysql_upgrade crashes if root's password contains an apostrophe/single quotation mark

Details

    Description

      Expected behaviour: mysql_upgrade completes successfully when using a password containing a single quotation mark (').

      Actual behaviour mysql_upgrade crashes with the following:

      root:~ # mysql_upgrade -p
      Enter password: 
      sh: -c: line 0: unexpected EOF while looking for matching `''
      sh: -c: line 1: syntax error: unexpected end of file
      Phase 1/3: Fixing table and database names
      sh: -c: line 0: unexpected EOF while looking for matching `''
      sh: -c: line 1: syntax error: unexpected end of file
      FATAL ERROR: Upgrade failed

      Attachments

        Issue Links

          Activity

            Also reproducible on all of MySQL 5.1-5.7

            elenst Elena Stepanova added a comment - Also reproducible on all of MySQL 5.1-5.7
            jb-boin Jean Weisbuch added a comment -

            Here is a patch for this bug :

            --- ./mysys/string.c	2014-09-25 00:29:46.000000000 +0200
            +++ ./mysys/string.c	2015-01-06 19:21:29.677750456 +0100
            @@ -145,13 +145,13 @@
               const char *quote_str= "\"";
               const uint  quote_len= 1;
             #else
            -  const char *quote_str= "\'";
            -  const uint  quote_len= 1;
            +  const char *quote_str= "\'\"\'\"\'";
            +  const uint  quote_len= 5;
             #endif /* __WIN__ */
               my_bool ret= TRUE;
               va_list dirty_text;
             
            -  ret&= dynstr_append_mem(str, quote_str, quote_len); /* Leading quote */
            +  ret&= dynstr_append_mem(str, quote_str, 1); /* Leading quote */
               va_start(dirty_text, append);
               while (append != NullS)
               {
            @@ -162,7 +162,9 @@
                 while(*(next_pos= strcend(cur_pos, quote_str[0])) != '\0')
                 {
                   ret&= dynstr_append_mem(str, cur_pos, (uint) (next_pos - cur_pos));
            -      ret&= dynstr_append_mem(str ,"\\", 1);
            +      #ifdef __WIN__
            +        ret&= dynstr_append_mem(str ,"\\", 1);
            +      #endif
                   ret&= dynstr_append_mem(str, quote_str, quote_len);
                   cur_pos= next_pos + 1;
                 }
            @@ -170,7 +172,7 @@
                 append= va_arg(dirty_text, char *);
               }
               va_end(dirty_text);
            -  ret&= dynstr_append_mem(str, quote_str, quote_len); /* Trailing quote */
            +  ret&= dynstr_append_mem(str, quote_str, 1); /* Trailing quote */
             
               return ret;
             }

            Before patching, if the provided password has a quote (maria'db on the example), the command executed using popen() on mysql_upgrade.c on the function run_command() looks like this :

            './mysql' '--no-defaults' '--port=3306' '--socket=/var/run/mysqld/mysqld.sock' '--user=*user defined in ~/.my.cnf*' '--password=*password defined in ~/.my.cnf*' '--socket=/var/run/mysqld/mysqld.sock' '--password=maria\'db' '--user=*user defined in ~/.my.cnf*'  '--database=mysql' '--batch' '--skip-force' '--silent' < /tmp/sqls8IHkR 2>&1

            The issue is that on a POSIX shell, you cannot escape anything on simple quoted string not even a simple quote, to do so you must close the simple quote, open a double quotes, put your simple quote on it then close the double quotes, for example :

            $ echo 'simple quotes: '"'"'here'"'"''
            simple quotes: 'here'

            After applying the patch, the resulting executed command looks like :

            './mysql' '--no-defaults' '--port=3306' '--socket=/var/run/mysqld/mysqld.sock' '--user=*user defined in ~/.my.cnf*' '--password=*password defined in ~/.my.cnf*' '--socket=/var/run/mysqld/mysqld.sock' '--password=maria'"'"'db' '--user=*user defined in ~/.my.cnf*'  '--database=mysql' '--batch' '--skip-force' '--silent' < /tmp/sqlG8Ftqz 2>&1

            I only tested the patch on Linux but to mimic the Windows behavior i inverted the quote_str and quote_len values from the Windows and Linux values and it worked fine too with double quotes instead of simple ones but i wasnt on a Windows OS so i am not 100% sure it wouldnt require a real test on it.
            The function seems to be also used on ./client/mysqltest.cc and ./libmysql/libmysql_exports_file.cc.

            ps: another small "bug" is that even if i didnt specify the user on the command line, it is passed twice on the mysql client.

            jb-boin Jean Weisbuch added a comment - Here is a patch for this bug : --- ./mysys/string.c 2014-09-25 00:29:46.000000000 +0200 +++ ./mysys/string.c 2015-01-06 19:21:29.677750456 +0100 @@ -145,13 +145,13 @@ const char *quote_str= "\""; const uint quote_len= 1; #else - const char *quote_str= "\'"; - const uint quote_len= 1; + const char *quote_str= "\'\"\'\"\'"; + const uint quote_len= 5; #endif /* __WIN__ */ my_bool ret= TRUE; va_list dirty_text; - ret&= dynstr_append_mem(str, quote_str, quote_len); /* Leading quote */ + ret&= dynstr_append_mem(str, quote_str, 1); /* Leading quote */ va_start(dirty_text, append); while (append != NullS) { @@ -162,7 +162,9 @@ while(*(next_pos= strcend(cur_pos, quote_str[0])) != '\0') { ret&= dynstr_append_mem(str, cur_pos, (uint) (next_pos - cur_pos)); - ret&= dynstr_append_mem(str ,"\\", 1); + #ifdef __WIN__ + ret&= dynstr_append_mem(str ,"\\", 1); + #endif ret&= dynstr_append_mem(str, quote_str, quote_len); cur_pos= next_pos + 1; } @@ -170,7 +172,7 @@ append= va_arg(dirty_text, char *); } va_end(dirty_text); - ret&= dynstr_append_mem(str, quote_str, quote_len); /* Trailing quote */ + ret&= dynstr_append_mem(str, quote_str, 1); /* Trailing quote */ return ret; } Before patching, if the provided password has a quote ( maria'db on the example), the command executed using popen() on mysql_upgrade.c on the function run_command() looks like this : './mysql' '--no-defaults' '--port=3306' '--socket=/var/run/mysqld/mysqld.sock' '--user=*user defined in ~/.my.cnf*' '--password=*password defined in ~/.my.cnf*' '--socket=/var/run/mysqld/mysqld.sock' '--password=maria\'db' '--user=*user defined in ~/.my.cnf*' '--database=mysql' '--batch' '--skip-force' '--silent' < /tmp/sqls8IHkR 2>&1 The issue is that on a POSIX shell, you cannot escape anything on simple quoted string not even a simple quote, to do so you must close the simple quote, open a double quotes, put your simple quote on it then close the double quotes, for example : $ echo 'simple quotes: '"'"'here'"'"'' simple quotes: 'here' After applying the patch, the resulting executed command looks like : './mysql' '--no-defaults' '--port=3306' '--socket=/var/run/mysqld/mysqld.sock' '--user=*user defined in ~/.my.cnf*' '--password=*password defined in ~/.my.cnf*' '--socket=/var/run/mysqld/mysqld.sock' '--password=maria'"'"'db' '--user=*user defined in ~/.my.cnf*' '--database=mysql' '--batch' '--skip-force' '--silent' < /tmp/sqlG8Ftqz 2>&1 I only tested the patch on Linux but to mimic the Windows behavior i inverted the quote_str and quote_len values from the Windows and Linux values and it worked fine too with double quotes instead of simple ones but i wasnt on a Windows OS so i am not 100% sure it wouldnt require a real test on it. The function seems to be also used on ./client/mysqltest.cc and ./libmysql/libmysql_exports_file.cc . ps: another small "bug" is that even if i didnt specify the user on the command line, it is passed twice on the mysql client.

            People

              serg Sergei Golubchik
              n3hima Joe MacMahon
              Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.