MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:October 23 2007 12:05pm
Subject:bk commit into 5.0 tree (davi:1.2543) BUG#31669
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of davi. When davi does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-10-23 09:05:39-03:00, davi@stripped +2 -0
  Bug#31669 Buffer overflow in mysql_change_user()
  
  The problem is that when copying the supplied username and
  database, no bounds checking is performed on the fixed-length
  buffer. A sufficiently large (> 512) user string can easily
  cause stack corruption. Since this API can be used from PHP
  and other programs, this is a serious problem.
  
  The solution is to increase the buffer size to the accepted
  size in similar functions and perform bounds checking when
  copying the username and database.

  libmysql/libmysql.c@stripped, 2007-10-23 09:05:37-03:00, davi@stripped +4 -3
    Increase the buffer size and perform bounds checking when copying
    the supplied arguments.

  tests/mysql_client_test.c@stripped, 2007-10-23 09:05:37-03:00, davi@stripped +94 -0
    Add test case for Bug#31669

diff -Nrup a/libmysql/libmysql.c b/libmysql/libmysql.c
--- a/libmysql/libmysql.c	2007-10-17 14:22:40 -03:00
+++ b/libmysql/libmysql.c	2007-10-23 09:05:37 -03:00
@@ -706,7 +706,8 @@ int cli_read_change_user_result(MYSQL *m
 my_bool	STDCALL mysql_change_user(MYSQL *mysql, const char *user,
 				  const char *passwd, const char *db)
 {
-  char buff[512],*end=buff;
+  char buff[USERNAME_LENGTH+SCRAMBLED_PASSWORD_CHAR_LENGTH+NAME_LEN+2];
+  char *end= buff;
   int rc;
   DBUG_ENTER("mysql_change_user");
 
@@ -716,7 +717,7 @@ my_bool	STDCALL mysql_change_user(MYSQL 
     passwd="";
 
   /* Store user into the buffer */
-  end=strmov(end,user)+1;
+  end= strmake(end, user, USERNAME_LENGTH) + 1;
 
   /* write scrambled password according to server capabilities */
   if (passwd[0])
@@ -736,7 +737,7 @@ my_bool	STDCALL mysql_change_user(MYSQL 
   else
     *end++= '\0';                               /* empty password */
   /* Add database if needed */
-  end= strmov(end, db ? db : "") + 1;
+  end= strmake(end, db ? db : "", NAME_LEN) + 1;
 
   /* Write authentication package */
   simple_command(mysql,COM_CHANGE_USER, buff,(ulong) (end-buff),1);
diff -Nrup a/tests/mysql_client_test.c b/tests/mysql_client_test.c
--- a/tests/mysql_client_test.c	2007-09-14 23:45:13 -03:00
+++ b/tests/mysql_client_test.c	2007-10-23 09:05:37 -03:00
@@ -15857,6 +15857,99 @@ static void test_bug29306()
   DBUG_VOID_RETURN;
 }
 
+
+/**
+  Bug#31669 Buffer overflow in mysql_change_user()
+*/
+
+#define LARGE_BUFFER_SIZE 2048
+
+static void test_bug31669()
+{
+  int rc;
+  static char buff[LARGE_BUFFER_SIZE+1];
+#ifndef EMBEDDED_LIBRARY
+  static char user[USERNAME_LENGTH+1];
+  static char db[NAME_LEN+1];
+  static char query[LARGE_BUFFER_SIZE*2];
+#endif
+
+  DBUG_ENTER("test_bug31669");
+  myheader("test_bug31669");
+
+  rc= mysql_change_user(mysql, NULL, NULL, NULL);
+  DIE_UNLESS(rc);
+
+  rc= mysql_change_user(mysql, "", "", "");
+  DIE_UNLESS(rc);
+
+  memset(buff, 'a', sizeof(buff));
+
+  rc= mysql_change_user(mysql, buff, buff, buff);
+  DIE_UNLESS(rc);
+
+  rc = mysql_change_user(mysql, opt_user, opt_password, current_db);
+  DIE_UNLESS(!rc);
+
+#ifndef EMBEDDED_LIBRARY
+  memset(db, 'a', sizeof(db));
+  db[NAME_LEN]= 0;
+  strxmov(query, "CREATE DATABASE IF NOT EXISTS ", db, NullS);
+  rc= mysql_query(mysql, query);
+  myquery(rc);
+
+  memset(user, 'b', sizeof(user));
+  user[USERNAME_LENGTH]= 0;
+  memset(buff, 'c', sizeof(buff));
+  buff[LARGE_BUFFER_SIZE]= 0;
+  strxmov(query, "GRANT ALL PRIVILEGES ON *.* TO '", user, "'@'%' IDENTIFIED BY "
+                 "'", buff, "' WITH GRANT OPTION", NullS);
+  rc= mysql_query(mysql, query);
+  myquery(rc);
+
+  rc= mysql_query(mysql, "FLUSH PRIVILEGES");
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user, buff, db);
+  DIE_UNLESS(!rc);
+
+  user[USERNAME_LENGTH-1]= 'a';
+  rc= mysql_change_user(mysql, user, buff, db);
+  DIE_UNLESS(rc);
+
+  user[USERNAME_LENGTH-1]= 'b';
+  buff[LARGE_BUFFER_SIZE-1]= 'd';
+  rc= mysql_change_user(mysql, user, buff, db);
+  DIE_UNLESS(rc);
+
+  buff[LARGE_BUFFER_SIZE-1]= 'c';
+  db[NAME_LEN-1]= 'e';
+  rc= mysql_change_user(mysql, user, buff, db);
+  DIE_UNLESS(rc);
+
+  db[NAME_LEN-1]= 'a';
+  rc= mysql_change_user(mysql, user, buff, db);
+  DIE_UNLESS(!rc);
+
+  rc= mysql_change_user(mysql, user + 1, buff + 1, db + 1);
+  DIE_UNLESS(rc);
+
+  rc = mysql_change_user(mysql, opt_user, opt_password, current_db);
+  DIE_UNLESS(!rc);
+
+  strxmov(query, "DROP DATABASE ", db, NullS);
+  rc= mysql_query(mysql, query);
+  myquery(rc);
+
+  strxmov(query, "DELETE FROM mysql.user WHERE User='", user, "'", NullS);
+  rc= mysql_query(mysql, query);
+  myquery(rc);
+  DIE_UNLESS(mysql_affected_rows(mysql) == 1);
+#endif
+
+  DBUG_VOID_RETURN;
+}
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -16149,6 +16242,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug27592", test_bug27592 },
   { "test_bug29948", test_bug29948 },
   { "test_bug29306", test_bug29306 },
+  { "test_bug31669", test_bug31669 },
   { 0, 0 }
 };
 
Thread
bk commit into 5.0 tree (davi:1.2543) BUG#31669Davi Arnaut23 Oct