mysql help sends unchecked contents to mysqld

From the MySQL Bug Report:

[15 Jun 14:28] Shane Bester

the problem seems to be that glob_buffer contains this incorrectly sprintf'd into the
pointer:

sprintf((char*) glob_buffer.ptr(),
   "Your MySQL connection id is %lu\nServer version: %s\n",
   mysql_thread_id(&mysql), server_version_string(&mysql));

Now further down in the code, the is_empty() method still believes the String
to be empty, but it's not.

it's not a security vulnerability, because the bug is in mysql - command line client - not on the server.
still it's a bug, that should be fixed.

It would be nice to have access to the bug report to see if we can get this fixed for oneiric.

Regards
chuck

First of all, this doesn't seem to be any sort of security vulnerability (not related to any stack overflow or any stack smashing etc.).It is something to do with parsing.

Also, not related to glob_buffer or it being empty as suggested above. (even in normal case it is like that).

The problem is in com_server_help:

static int com_server_help(String *buffer __attribute__((unused)),
      char *line __attribute__((unused)), char *help_arg)
{
  MYSQL_ROW cur;
  const char *server_cmd= buffer->ptr();
  char cmd_buf[100 + 1];
  MYSQL_RES *result;
  int error;

  if (help_arg[0] != '\'')
  {
 char *end_arg= strend(help_arg);
 if(--end_arg)
 {
  while (my_isspace(charset_info,*end_arg))
          end_arg--;
  *++end_arg= '\0';
 }
 (void) strxnmov(cmd_buf, sizeof(cmd_buf), "help '", help_arg, "'", NullS);
    server_cmd= cmd_buf;
  }

  if (!status.batch)
  {
    old_buffer= *buffer;
    old_buffer.copy();
  }
======

As you can see it explicitly checks for single quote and does some string filtering to finally append " help ' " and " ' " to it if does not have them already.

The problem lies here --
  const char *server_cmd= buffer->ptr()

If the string already starts with single quote, server_cmd ends up with value of glob_buffer like this:

 print server_cmd
$10 = 0x98d660 "Your MySQL connection id is 11\nServer version: 5.5.27-rel28.0-debug-log Built by raghavendra at Tue Aug 21 00:41:10 IST 2012\n"

and rest follows.

Interesting to observe that the argument has been marked __attribute__((unused)) but is still used.

This section
===
  if (!status.batch)
  {
    old_buffer= *buffer;
    old_buffer.copy();
  }
======

is also suspicious (because of unused attribute) but *not* directly relevant to this bug. (For curious, old_buffer is used in com_edit when \e is invoked, however, after the fix(below) I checked and \e along with \h was working fine: something like

 > select \h help 'contents' \e will copy select to $EDITOR's buffer

Anyways, here is the fix:

=== modified file 'Percona-Server/client/mysql.cc'
--- Percona-Server/client/mysql.cc 2012-08-07 06:10:00 +0000
+++ Percona-Server/client/mysql.cc 2012-09-05 16:14:14 +0000
@@ -2827,7 +2827,7 @@
                           char *line __attribute__((unused)), char *help_arg)
 {
   MYSQL_ROW cur;
- const char *server_cmd= buffer->ptr();
+ const char *server_cmd= help_arg;
   char cmd_buf[100 + 1];
   MYSQL_RES *result;
   int error;
@@ -2842,8 +2842,10 @@
                *++end_arg= '\0';
        }
        (void) strxnmov(cmd_buf, sizeof(cmd_buf), "help '", help_arg, "'", NullS);
- server_cmd= cmd_buf;
+ } else {
+ (void) strxnmov(cmd_buf, sizeof(cmd_buf), "help ", help_arg, NullS);
   }
+ server_cmd= cmd_buf;

After the fix:

>>./client/mysql
Welcome to the MySQL monitor. Commands end with ; or \g.
Your MySQL connection id is 19
Server version: 5.5.27-log Source distribution

Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current i...

Regarding comment 1,

print glob_buffer
$6 = {Ptr = 0x98d660 "Your MySQL connection id is 11\nServer version: 5.5.27-rel28.0-debug-log Built by raghavendra at Tue Aug 21 00:41:10 IST 2012\n", str_length = 0, Alloced_length = 520, alloced = true, str_charset = 0x8cfb20 <my_charset_bin>}

For some reason, str_length shows up as zero. However, I think it is something to do with String class used sql_string.h. Anyways, even in normal cases, it is like that, shouldn't be related to this.

https://mariadb.atlassian.net/browse/MDEV-687

Oracle fixed it in 5.1.66 and 5.5.28. Closing.

percona-server/5.1$ bzr log -r 0.15786.4
------------------------------------------------------------
revno: 0.15786.4
committer: Venkata Sidagam <email address hidden>
branch nick: mysql-5.1-13955256
timestamp: Thu 2012-07-19 13:52:34 +0530
message:
  Bug #12615411 - server side help doesn't work as first statement

  Problem description:
  Giving "help 'contents'" in the mysql client as a first statement
  gives error

  Analysis:
  In com_server_help() function the "server_cmd" variable was
  initialised with buffer->ptr(). And the "server_cmd" variable is not
  updated since we are passing "'contents'"(with single quote) so the
  buffer->ptr() consists of the previous buffer values and it was sent
  to the mysql_real_query() hence we are getting error.

  Fix:
  We are not initialising the "server_cmd" variable and we are updating
  the variable with "server_cmd= cmd_buf" in any of the case i.e with
  single quote or without single quote for the contents.
  As part of error message improvement, added new error message in case
  of "help 'contents'".

And in 5.6.7.

Closing for MySQL 5.1 as this has already been addressed by Oracle

Also marking Maria as invalid, as MariaDB does not use launchpad to track its bugs

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PS-1206