List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:March 29 2007 12:21pm
Subject:bk commit into 5.0 tree (anozdrin:1.2423) BUG#27337
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of alik. When alik 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-03-29 16:21:50+04:00, anozdrin@stripped +5 -0
  Fix for BUG#27337: Privileges are not properly restored.
  
  The problem was that THD::db_access variable was not restored after
  database switch in stored-routine-execution code.
  
  The fix is to restore THD::db_access in this case.
  
  Unfortunately, this fix requires additional changes,
  because in prepare_schema_table(), called on the parsing stage, we checked
  privileges. That was wrong according to our design, but this flaw haven't
  struck so far, because it was masked. All privilege checkings must be
  done on the execution stage in order to be compatible with prepared statements
  and stored routines. So, this patch also contains patch for
  prepare_schema_table(), which moves the checkings to the execution phase.

  mysql-test/r/grant.result@stripped, 2007-03-29 16:21:42+04:00, anozdrin@stripped +28 -0
    Updated result file.

  mysql-test/t/grant.test@stripped, 2007-03-29 16:21:43+04:00, anozdrin@stripped +59 -0
    Added test case for BUG#27337.

  sql/sql_db.cc@stripped, 2007-03-29 16:21:43+04:00, anozdrin@stripped +20 -23
    Fix for BUG#27337 -- set THD::db_access even if we're called
    from stored-routine-execution code.

  sql/sql_parse.cc@stripped, 2007-03-29 16:21:43+04:00, anozdrin@stripped +112 -52
    Split prepare_schema_table() into two functions:
      - prepare_schema_table(), which is called from the parser (parsing stage);
      - check_show_access(), which is called on the execution stage.

  sql/table.h@stripped, 2007-03-29 16:21:43+04:00, anozdrin@stripped +7 -0
    Added necessary attributes to split prepare_schema_table().

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	anozdrin
# Host:	booka.opbmk
# Root:	/home/alik/Documents/MySQL/devel/5.0-marvel-27337

--- 1.135/sql/sql_db.cc	2007-03-27 21:54:59 +04:00
+++ 1.136/sql/sql_db.cc	2007-03-29 16:21:43 +04:00
@@ -1308,30 +1308,27 @@
   DBUG_PRINT("info",("Use database: %s", new_db_file_name.str));
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-  if (!force_switch) /* FIXME: this is BUG#27337. */
-  {
-    db_access=
-      test_all_bits(sctx->master_access, DB_ACLS) ?
-      DB_ACLS :
-      acl_get(sctx->host,
-              sctx->ip,
-              sctx->priv_user,
-              new_db_file_name.str,
-              FALSE) | sctx->master_access;
+  db_access=
+    test_all_bits(sctx->master_access, DB_ACLS) ?
+    DB_ACLS :
+    acl_get(sctx->host,
+            sctx->ip,
+            sctx->priv_user,
+            new_db_file_name.str,
+            FALSE) | sctx->master_access;
 
-    if (!force_switch &&
-        !(db_access & DB_ACLS) &&
-        (!grant_option || check_grant_db(thd, new_db_file_name.str)))
-    {
-      my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
-               sctx->priv_user,
-               sctx->priv_host,
-               new_db_file_name.str);
-      mysql_log.write(thd, COM_INIT_DB, ER(ER_DBACCESS_DENIED_ERROR),
-                      sctx->priv_user, sctx->priv_host, new_db_file_name.str);
-      my_free(new_db_file_name.str, MYF(0));
-      DBUG_RETURN(TRUE);
-    }
+  if (!force_switch &&
+      !(db_access & DB_ACLS) &&
+      (!grant_option || check_grant_db(thd, new_db_file_name.str)))
+  {
+    my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
+             sctx->priv_user,
+             sctx->priv_host,
+             new_db_file_name.str);
+    mysql_log.write(thd, COM_INIT_DB, ER(ER_DBACCESS_DENIED_ERROR),
+                    sctx->priv_user, sctx->priv_host, new_db_file_name.str);
+    my_free(new_db_file_name.str, MYF(0));
+    DBUG_RETURN(TRUE);
   }
 #endif
 

--- 1.615/sql/sql_parse.cc	2007-03-27 21:54:59 +04:00
+++ 1.616/sql/sql_parse.cc	2007-03-29 16:21:43 +04:00
@@ -2238,6 +2238,9 @@
 {
   DBUG_ENTER("prepare_schema_table");
   SELECT_LEX *sel= 0;
+  char *dst_db_name= NULL;
+  TABLE_LIST *dst_table= NULL;
+
   switch (schema_table_idx) {
   case SCH_SCHEMATA:
 #if defined(DONT_ALLOW_SHOW_COMMANDS)
@@ -2245,11 +2248,9 @@
                ER(ER_NOT_ALLOWED_COMMAND), MYF(0));   /* purecov: inspected */
     DBUG_RETURN(1);
 #else
-    if ((specialflag & SPECIAL_SKIP_SHOW_DB) &&
-	check_global_access(thd, SHOW_DB_ACL))
-      DBUG_RETURN(1);
     break;
 #endif
+
   case SCH_TABLE_NAMES:
   case SCH_TABLES:
   case SCH_VIEWS:
@@ -2259,32 +2260,22 @@
                ER(ER_NOT_ALLOWED_COMMAND), MYF(0)); /* purecov: inspected */
     DBUG_RETURN(1);
 #else
+    if (lex->select_lex.db == NULL &&
+        thd->copy_db_to(&lex->select_lex.db, 0))
     {
-      char *db;
-      if (lex->select_lex.db == NULL &&
-          thd->copy_db_to(&lex->select_lex.db, 0))
-      {
-        DBUG_RETURN(1);
-      }
-      db= lex->select_lex.db;
-      remove_escape(db);				// Fix escaped '_'
-      if (check_db_name(db))
-      {
-        my_error(ER_WRONG_DB_NAME, MYF(0), db);
-        DBUG_RETURN(1);
-      }
-      if (check_access(thd, SELECT_ACL, db, &thd->col_access, 0, 0,
-                       is_schema_db(db)))
-        DBUG_RETURN(1);			        /* purecov: inspected */
-      if (!thd->col_access && check_grant_db(thd,db))
-      {
-	my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
-                 thd->security_ctx->priv_user, thd->security_ctx->priv_host,
-                 db);
-	DBUG_RETURN(1);
-      }
-      break;
+      DBUG_RETURN(1);
     }
+
+    dst_db_name= lex->select_lex.db;
+    remove_escape(dst_db_name); // Fix escaped '_'
+
+    if (check_db_name(dst_db_name))
+    {
+      my_error(ER_WRONG_DB_NAME, MYF(0), dst_db_name);
+      DBUG_RETURN(1);
+    }
+
+    break;
 #endif
   case SCH_COLUMNS:
   case SCH_STATISTICS:
@@ -2293,30 +2284,23 @@
                ER(ER_NOT_ALLOWED_COMMAND), MYF(0)); /* purecov: inspected */
     DBUG_RETURN(1);
 #else
-    if (table_ident)
-    {
-      TABLE_LIST **query_tables_last= lex->query_tables_last;
-      sel= new SELECT_LEX();
-      /* 'parent_lex' is used in init_query() so it must be before it. */
-      sel->parent_lex= lex;
-      sel->init_query();
-      if (!sel->add_table_to_list(thd, table_ident, 0, 0, TL_READ, 
-                                 (List<String> *) 0, (List<String> *) 0))
-        DBUG_RETURN(1);
-      lex->query_tables_last= query_tables_last;
-      TABLE_LIST *table_list= (TABLE_LIST*) sel->table_list.first;
-      char *db= table_list->db;
-      remove_escape(db);			// Fix escaped '_'
-      remove_escape(table_list->table_name);
-      if (check_access(thd,SELECT_ACL | EXTRA_ACL,db,
-                       &table_list->grant.privilege, 0, 0,
-                       test(table_list->schema_table)))
-        DBUG_RETURN(1);				/* purecov: inspected */
-      if (grant_option && check_grant(thd, SELECT_ACL, table_list, 2,
-                                      UINT_MAX, 0))
-        DBUG_RETURN(1);
-      break;
-    }
+    DBUG_ASSERT(table_ident);
+
+    TABLE_LIST **query_tables_last= lex->query_tables_last;
+    sel= new SELECT_LEX();
+    /* 'parent_lex' is used in init_query() so it must be before it. */
+    sel->parent_lex= lex;
+    sel->init_query();
+    if (!sel->add_table_to_list(thd, table_ident, 0, 0, TL_READ, 
+                               (List<String> *) 0, (List<String> *) 0))
+      DBUG_RETURN(1);
+    lex->query_tables_last= query_tables_last;
+
+    dst_table= (TABLE_LIST*) sel->table_list.first;
+    remove_escape(dst_table->db);			// Fix escaped '_'
+    remove_escape(dst_table->table_name);
+
+    break;
 #endif
   case SCH_OPEN_TABLES:
   case SCH_VARIABLES:
@@ -2343,6 +2327,9 @@
   TABLE_LIST *table_list= (TABLE_LIST*) select_lex->table_list.first;
   table_list->schema_select_lex= sel;
   table_list->schema_table_reformed= 1;
+  table_list->schema_table_idx= schema_table_idx;
+  table_list->show_dst_db_name= dst_db_name;
+  table_list->show_dst_table= dst_table;
   statistic_increment(thd->status_var.com_stat[lex->orig_sql_command],
                       &LOCK_status);
   DBUG_RETURN(0);
@@ -5390,6 +5377,70 @@
 }
 
 
+static bool check_show_access(THD *thd, TABLE_LIST *table)
+{
+  switch (table->schema_table_idx)
+  {
+  case SCH_SCHEMATA:
+    return (specialflag & SPECIAL_SKIP_SHOW_DB) &&
+           check_global_access(thd, SHOW_DB_ACL);
+
+  case SCH_TABLE_NAMES:
+  case SCH_TABLES:
+  case SCH_VIEWS:
+  case SCH_TRIGGERS:
+    DBUG_ASSERT(table->show_dst_db_name);
+
+    if (check_access(thd, SELECT_ACL, table->show_dst_db_name,
+                     &thd->col_access, FALSE, FALSE,
+                     is_schema_db(table->show_dst_db_name)))
+    {
+      return TRUE;
+    }
+
+    if (!thd->col_access && check_grant_db(thd, table->show_dst_db_name))
+    {
+      my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
+               thd->security_ctx->priv_user, thd->security_ctx->priv_host,
+               table->show_dst_db_name);
+      return TRUE;
+    }
+
+    return FALSE;
+
+  case SCH_COLUMNS:
+  case SCH_STATISTICS:
+    DBUG_ASSERT(table->show_dst_table);
+
+    return check_access(thd, SELECT_ACL | EXTRA_ACL,
+                        table->show_dst_table->db,
+                        &table->show_dst_table->grant.privilege,
+                        0, 0,
+                        test(table->show_dst_table->schema_table)) ||
+           grant_option && check_grant(thd, SELECT_ACL,
+                                       table->show_dst_table,
+                                       2, UINT_MAX, 0);
+
+  case SCH_OPEN_TABLES:
+  case SCH_VARIABLES:
+  case SCH_STATUS:
+  case SCH_PROCEDURES:
+  case SCH_CHARSETS:
+  case SCH_COLLATIONS:
+  case SCH_COLLATION_CHARACTER_SET_APPLICABILITY:
+  case SCH_USER_PRIVILEGES:
+  case SCH_SCHEMA_PRIVILEGES:
+  case SCH_TABLE_PRIVILEGES:
+  case SCH_COLUMN_PRIVILEGES:
+  case SCH_TABLE_CONSTRAINTS:
+  case SCH_KEY_COLUMN_USAGE:
+    break;
+  }
+
+  return FALSE;
+}
+
+
 /*
   Check the privilege for all used tables.
 
@@ -5450,7 +5501,16 @@
        Remove SHOW_VIEW_ACL, because it will be checked during making view
      */
     tables->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
-    if (tables->derived || tables->schema_table ||
+
+    if (tables->schema_table_reformed)
+    {
+      if (check_show_access(thd, tables))
+        goto deny;
+
+      continue;
+    }
+
+    if (tables->derived ||
         (tables->table && (int)tables->table->s->tmp_table) ||
         my_tz_check_n_skip_implicit_tables(&tables,
                                            thd->lex->time_zone_tables_used))

--- 1.139/sql/table.h	2007-03-06 21:19:14 +03:00
+++ 1.140/sql/table.h	2007-03-29 16:21:43 +04:00
@@ -534,6 +534,13 @@
   st_table_list	*correspondent_table;
   st_select_lex_unit *derived;		/* SELECT_LEX_UNIT of derived table */
   ST_SCHEMA_TABLE *schema_table;        /* Information_schema table */
+
+  /* The following three attributes are used for SHOW statements. */
+
+  enum_schema_tables schema_table_idx;
+  char *show_dst_db_name;
+  st_table_list *show_dst_table;
+
   st_select_lex	*schema_select_lex;
   /*
     True when the view field translation table is used to convert

--- 1.62/mysql-test/r/grant.result	2007-03-23 14:12:09 +03:00
+++ 1.63/mysql-test/r/grant.result	2007-03-29 16:21:42 +04:00
@@ -1019,4 +1019,32 @@
 DROP DATABASE mysqltest3;
 DROP DATABASE mysqltest4;
 DROP USER mysqltest_1@localhost;
+DROP DATABASE IF EXISTS mysqltest1;
+DROP DATABASE IF EXISTS mysqltest2;
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+GRANT ALL PRIVILEGES ON mysqltest1.* TO mysqltest_1@localhost;
+GRANT SELECT ON mysqltest2.* TO mysqltest_1@localhost;
+CREATE PROCEDURE mysqltest1.p1() SQL SECURITY INVOKER
+SELECT 1;
+
+---> connection: bug27337_con1
+CREATE TABLE t1(c INT);
+ERROR 42000: CREATE command denied to user 'mysqltest_1'@'localhost' for table 't1'
+CALL mysqltest1.p1();
+1
+1
+CREATE TABLE t1(c INT);
+ERROR 42000: CREATE command denied to user 'mysqltest_1'@'localhost' for table 't1'
+
+---> connection: bug27337_con2
+CREATE TABLE t1(c INT);
+ERROR 42000: CREATE command denied to user 'mysqltest_1'@'localhost' for table 't1'
+SHOW TABLES;
+Tables_in_mysqltest2
+
+---> connection: default
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+DROP USER mysqltest_1@localhost;
 End of 5.0 tests

--- 1.52/mysql-test/t/grant.test	2007-03-23 14:12:09 +03:00
+++ 1.53/mysql-test/t/grant.test	2007-03-29 16:21:43 +04:00
@@ -958,4 +958,63 @@
 DROP USER mysqltest_1@localhost;
 
 
+#
+# BUG#27337: Privileges are not restored properly.
+#
+
+# Prepare.
+
+--disable_warnings
+DROP DATABASE IF EXISTS mysqltest1;
+DROP DATABASE IF EXISTS mysqltest2;
+--enable_warnings
+
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+
+GRANT ALL PRIVILEGES ON mysqltest1.* TO mysqltest_1@localhost;
+GRANT SELECT ON mysqltest2.* TO mysqltest_1@localhost;
+
+CREATE PROCEDURE mysqltest1.p1() SQL SECURITY INVOKER
+  SELECT 1;
+
+# Test.
+
+--connect (bug27337_con1,localhost,mysqltest_1,,mysqltest2)
+--echo
+--echo ---> connection: bug27337_con1
+
+--error ER_TABLEACCESS_DENIED_ERROR
+CREATE TABLE t1(c INT);
+
+CALL mysqltest1.p1();
+
+--error ER_TABLEACCESS_DENIED_ERROR
+CREATE TABLE t1(c INT);
+
+--disconnect bug27337_con1
+
+--connect (bug27337_con2,localhost,mysqltest_1,,mysqltest2)
+--echo
+--echo ---> connection: bug27337_con2
+
+--error ER_TABLEACCESS_DENIED_ERROR
+CREATE TABLE t1(c INT);
+
+SHOW TABLES;
+
+# Cleanup.
+
+--connection default
+--echo
+--echo ---> connection: default
+
+--disconnect bug27337_con2
+
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+
+DROP USER mysqltest_1@localhost;
+
+
 --echo End of 5.0 tests
Thread
bk commit into 5.0 tree (anozdrin:1.2423) BUG#27337Alexander Nozdrin29 Mar