Ensure that privileges are tested properly for multi-table-updates.

Now one need only SELECT privilege for tables that are only read in
UPDATE statements with many tables. (Bug #2377).


sql/sql_acl.cc:
  Comment cleanup
sql/sql_parse.cc:
  Merged duplicate code.
  Removed some outdated 'tables->db' tests.
  Check privileges for multi-updates properly (Bug #2377)
sql/sql_show.cc:
  Remove disabled code
sql/sql_update.cc:
  Ensure that privileges are tested properly for multi-table-updates
tests/grant.pl:
  Added more tests
tests/grant.res:
  updated results

sql/sql_acl.cc

@@ -2539,7 +2539,7 @@ void grant_reload(THD *thd)
 
 /****************************************************************************
   Check grants
-  All errors are written directly to the client if command name is given !
+  All errors are written directly to the client if no_errors is given !
 ****************************************************************************/
 
 bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,

sql/sql_parse.cc

@@ -63,6 +63,8 @@ static bool append_file_to_dir(THD *thd, const char **filename_ptr,
 			       const char *table_name);
 static bool create_total_list(THD *thd, LEX *lex,
 			      TABLE_LIST **result, bool skip_first);
+static bool check_one_table_access(THD *thd, ulong want_access,
+				   TABLE_LIST *table, bool no_errors);
 
 const char *any_db="*any*";	// Special symbol for check_access
 
@@ -870,11 +872,8 @@ int mysql_table_dump(THD* thd, char* db, char* tbl_name, int fd)
   if (!(table=open_ltable(thd, table_list, TL_READ_NO_INSERT)))
     DBUG_RETURN(1);
 
-  if (check_access(thd, SELECT_ACL, db, &table_list->grant.privilege))
+  if (check_one_table_access(thd, SELECT_ACL, table_list, 0))
     goto err;
-  if (grant_option && check_grant(thd, SELECT_ACL, table_list))
-    goto err;
-
   thd->free_list = 0;
   thd->query_length=(uint) strlen(tbl_name);
   thd->query = tbl_name;
@@ -1102,9 +1101,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       casedn_str(table_list.real_name);
     remove_escape(table_list.real_name);	// This can't have wildcards
 
-    if (check_access(thd,SELECT_ACL,table_list.db,&thd->col_access))
+    if (check_access(thd,SELECT_ACL,table_list.db,&table_list.grant.privilege))
       break;
-    table_list.grant.privilege=thd->col_access;
     if (grant_option && check_grant(thd,SELECT_ACL,&table_list,2))
       break;
     mysqld_list_fields(thd,&table_list,fields);
@@ -1693,12 +1691,8 @@ mysql_execute_command(void)
     break;
   }
   case SQLCOM_CREATE_INDEX:
-    if (!tables->db)
-      tables->db=thd->db;
-    if (check_access(thd,INDEX_ACL,tables->db,&tables->grant.privilege))
+    if (check_one_table_access(thd, INDEX_ACL, tables, 0))
       goto error; /* purecov: inspected */
-    if (grant_option && check_grant(thd,INDEX_ACL,tables))
-      goto error;
     if (end_active_trans(thd))
       res= -1;
     else
@@ -1753,8 +1747,6 @@ mysql_execute_command(void)
 	res=0;
 	break;
       }
-      if (!tables->db)
-	tables->db=thd->db;
       if (!select_lex->db)
 	select_lex->db=tables->db;
       if (check_access(thd,ALTER_ACL,tables->db,&tables->grant.privilege) ||
@@ -1763,8 +1755,6 @@ mysql_execute_command(void)
 				   (TABLE_LIST *)
 				   lex->create_info.merge_list.first))
 	goto error;				/* purecov: inspected */
-      if (!tables->db)
-	tables->db=thd->db;
       if (grant_option)
       {
 	if (check_grant(thd,ALTER_ACL,tables))
@@ -1909,16 +1899,8 @@ mysql_execute_command(void)
     break;
   }
   case SQLCOM_UPDATE:
-    TABLE_LIST *table;
     if (check_db_used(thd,tables))
       goto error;
-    for (table=tables ; table ; table=table->next)
-    {
-      if (check_access(thd,UPDATE_ACL,table->db,&table->grant.privilege))
-	goto error;
-    }
-    if (grant_option && check_grant(thd,UPDATE_ACL,tables))
-      goto error;
     if (select_lex->item_list.elements != lex->value_list.elements)
     {
       send_error(&thd->net,ER_WRONG_VALUE_COUNT);
@@ -1926,6 +1908,10 @@ mysql_execute_command(void)
     }
     if (select_lex->table_list.elements == 1)
     {
+      if (check_one_table_access(thd, UPDATE_ACL, tables, 0))
+	goto error; /* purecov: inspected */
+
+
       res= mysql_update(thd,tables,
 			select_lex->item_list,
 			lex->value_list,
@@ -1937,7 +1923,22 @@ mysql_execute_command(void)
     else
     {
       const char *msg= 0;
+      TABLE_LIST *table;
       lex->sql_command= SQLCOM_MULTI_UPDATE;
+
+      /*
+	Ensure that we have UPDATE or SELECT privilege for each table
+	The exact privilege is checked in mysql_multi_update()
+      */
+      for (table= tables ; table ; table= table->next)
+      {
+	TABLE_LIST *save= table->next;
+	table->next= 0;
+	if (check_one_table_access(thd, UPDATE_ACL, table, 1) &&
+	    check_one_table_access(thd, SELECT_ACL, table, 0))
+	    goto error;
+	table->next= save;
+      }
       if (select_lex->order_list.elements)
 	msg="ORDER BY";
       else if (select_lex->select_limit && select_lex->select_limit !=
@@ -1958,21 +1959,14 @@ mysql_execute_command(void)
     }
     break;
   case SQLCOM_INSERT:
-    if (check_access(thd,INSERT_ACL,tables->db,&tables->grant.privilege))
+    if (check_one_table_access(thd, INSERT_ACL, tables, 0))
       goto error; /* purecov: inspected */
-    if (grant_option && check_grant(thd,INSERT_ACL,tables))
-      goto error;
     res = mysql_insert(thd,tables,lex->field_list,lex->many_values,
 		       lex->duplicates);
     break;
   case SQLCOM_REPLACE:
-    if (check_access(thd,INSERT_ACL | DELETE_ACL,
-		     tables->db,&tables->grant.privilege))
+    if (check_one_table_access(thd, INSERT_ACL | DELETE_ACL, tables, 0))
       goto error; /* purecov: inspected */
-    if (grant_option && check_grant(thd,INSERT_ACL | DELETE_ACL,
-				    tables))
-
-      goto error;
     res = mysql_insert(thd,tables,lex->field_list,lex->many_values,
 		       DUP_REPLACE);
     break;
@@ -1988,9 +1982,7 @@ mysql_execute_command(void)
 			INSERT_ACL : INSERT_ACL | DELETE_ACL);
       TABLE_LIST *save_next=tables->next;
       tables->next=0;
-      if (check_access(thd, privilege,
-		       tables->db,&tables->grant.privilege) ||
-	  (grant_option && check_grant(thd, privilege, tables)))
+      if (check_one_table_access(thd, privilege, tables, 0))
 	goto error;
       tables->next=save_next;
       if ((res=check_table_access(thd, SELECT_ACL, save_next)))
@@ -2035,10 +2027,8 @@ mysql_execute_command(void)
     break;
   }
   case SQLCOM_TRUNCATE:
-    if (check_access(thd,DELETE_ACL,tables->db,&tables->grant.privilege))
+    if (check_one_table_access(thd, DELETE_ACL, tables, 0))
       goto error; /* purecov: inspected */
-    if (grant_option && check_grant(thd,DELETE_ACL,tables))
-      goto error;
     /*
       Don't allow this within a transaction because we want to use
       re-generate table
@@ -2052,9 +2042,7 @@ mysql_execute_command(void)
     break;
   case SQLCOM_DELETE:
   {
-    if (check_access(thd,DELETE_ACL,tables->db,&tables->grant.privilege))
-      goto error; /* purecov: inspected */
-    if (grant_option && check_grant(thd,DELETE_ACL,tables))
+    if (check_one_table_access(thd, DELETE_ACL, tables, 0))
       goto error;
     // Set privilege for the WHERE clause
     tables->grant.want_privilege=(SELECT_ACL & ~tables->grant.privilege);
@@ -2139,8 +2127,8 @@ mysql_execute_command(void)
       DROP / * 40005 TEMPORARY * / TABLE
       that come from parts of binlogs (likely if we use RESET SLAVE or CHANGE
       MASTER TO), while the temporary table has already been dropped.
-      To not generate such irrelevant "table does not exist errors", we silently
-      add IF EXISTS if TEMPORARY was used.
+      To not generate such irrelevant "table does not exist errors",
+      we silently add IF EXISTS if TEMPORARY was used.
     */
     if (thd->slave_thread && lex->drop_temporary)
       lex->drop_if_exists= 1;
@@ -2151,12 +2139,8 @@ mysql_execute_command(void)
   }
   break;
   case SQLCOM_DROP_INDEX:
-    if (!tables->db)
-      tables->db=thd->db;
-    if (check_access(thd,INDEX_ACL,tables->db,&tables->grant.privilege))
-      goto error;				/* purecov: inspected */
-    if (grant_option && check_grant(thd,INDEX_ACL,tables))
-      goto error;
+    if (check_one_table_access(thd, INDEX_ACL, tables, 0))
+      goto error; /* purecov: inspected */
     if (end_active_trans(thd))
       res= -1;
     else
@@ -2242,16 +2226,11 @@ mysql_execute_command(void)
 #else
     {
       char *db=tables->db;
-      if (!*db)
-      {
-	send_error(&thd->net,ER_NO_DB_ERROR);	/* purecov: inspected */
-	goto error;				/* purecov: inspected */
-      }
       remove_escape(db);			// Fix escaped '_'
       remove_escape(tables->real_name);
-      if (check_access(thd,SELECT_ACL | EXTRA_ACL,db,&thd->col_access))
+      if (check_access(thd,SELECT_ACL | EXTRA_ACL,db,
+		       &tables->grant.privilege))
 	goto error;				/* purecov: inspected */
-      tables->grant.privilege=thd->col_access;
       if (grant_option && check_grant(thd,SELECT_ACL,tables,2))
 	goto error;
       res= mysqld_show_fields(thd,tables,
@@ -2267,18 +2246,10 @@ mysql_execute_command(void)
 #else
     {
       char *db=tables->db;
-      if (!db)
-      {
-	send_error(&thd->net,ER_NO_DB_ERROR);	/* purecov: inspected */
-	goto error;				/* purecov: inspected */
-      }
       remove_escape(db);			// Fix escaped '_'
       remove_escape(tables->real_name);
-      if (!tables->db)
-	tables->db=thd->db;
-      if (check_access(thd,SELECT_ACL,db,&thd->col_access))
+      if (check_access(thd,SELECT_ACL,db,&tables->grant.privilege))
 	goto error; /* purecov: inspected */
-      tables->grant.privilege=thd->col_access;
       if (grant_option && check_grant(thd,SELECT_ACL,tables,2))
 	goto error;
       res= mysqld_show_keys(thd,tables);
@@ -2306,8 +2277,7 @@ mysql_execute_command(void)
 	send_error(&thd->net,ER_NOT_ALLOWED_COMMAND);
 	goto error;
       }
-      if (check_access(thd,privilege,tables->db,&tables->grant.privilege) ||
-	  grant_option && check_grant(thd,privilege,tables))
+      if (check_one_table_access(thd, privilege, tables, 0))
 	goto error;
     }
     res=mysql_load(thd, lex->exchange, tables, lex->field_list,
@@ -2805,6 +2775,18 @@ check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
 }
 
 
+static bool
+check_one_table_access(THD *thd, ulong want_access, TABLE_LIST *table,
+		       bool no_errors)
+{
+  if (check_access(thd, want_access, table->db, &table->grant.privilege, 0,
+		   no_errors))
+    return 1;
+  return (grant_option && check_grant(thd, want_access, table, 0,
+				      no_errors));
+}
+
+
 static bool check_db_used(THD *thd,TABLE_LIST *tables)
 {
   for (; tables ; tables=tables->next)

sql/sql_show.cc

@@ -491,11 +491,6 @@ mysqld_show_fields(THD *thd, TABLE_LIST *table_list,const char *wild,
   {
     if (!wild || !wild[0] || !wild_case_compare(field->field_name,wild))
     {
-#ifdef NOT_USED
-      if (thd->col_access & TABLE_ACLS ||
-          ! check_grant_column(thd,table,field->field_name,
-                               (uint) strlen(field->field_name),1))
-#endif
       {
         byte *pos;
         uint flags=field->flags;

sql/sql_update.cc

@@ -15,8 +15,9 @@
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
 
 
-/* Update of records 
-   Multi-table updates were introduced by Monty and Sinisa <sinisa@mysql.com>
+/*
+  Single table and multi table updates of tables.
+  Multi-table updates were introduced by Sinisa & Monty
 */
 
 #include "mysql_priv.h"
@@ -398,20 +399,33 @@ int mysql_multi_update(THD *thd,
   TABLE_LIST *tl;
   DBUG_ENTER("mysql_multi_update");
 
-  table_list->grant.want_privilege=(SELECT_ACL & ~table_list->grant.privilege);
   if ((res=open_and_lock_tables(thd,table_list)))
     DBUG_RETURN(res);
 
   thd->select_limit=HA_POS_ERROR;
+
+  /*
+    Ensure that we have update privilege for all tables and columns in the
+    SET part
+  */
+  for (tl= table_list ; tl ; tl=tl->next)
+  {
+    TABLE *table= tl->table;
+    table->grant.want_privilege= (UPDATE_ACL & ~table->grant.privilege);
+  }
+
   if (setup_fields(thd, table_list, *fields, 1, 0, 0))
     DBUG_RETURN(-1);
 
   /*
     Count tables and setup timestamp handling
   */
-  for (tl= (TABLE_LIST*) table_list ; tl ; tl=tl->next)
+  for (tl= table_list ; tl ; tl=tl->next)
   {
     TABLE *table= tl->table;
+
+    /* We only need SELECT privilege for columns in the values list */
+    table->grant.want_privilege= (SELECT_ACL & ~table->grant.privilege);
     if (table->timestamp_field)
     {
       table->time_stamp=0;

tests/grant.pl

@@ -54,7 +54,7 @@ safe_query("delete from columns_priv");
 safe_query("lock tables mysql.user write"); # Test lock tables
 safe_query("flush privileges");
 safe_query("unlock tables");	     # should already be unlocked
-safe_query("drop database $opt_database",2);
+safe_query("drop database $opt_database",3);	# Don't print possible error
 safe_query("create database $opt_database");
 
 # check that the user can't login yet
@@ -186,6 +186,7 @@ user_query("delete from $opt_database.test where a=3");
 user_query("create table $opt_database.test2 (a int not null)");
 user_query("alter table $opt_database.test2 add b int");
 user_query("create index dummy on $opt_database.test2 (a)");
+user_query("update test,test2 SET test.a=test2.a where test.a=test2.a");
 user_query("drop table $opt_database.test2");
 user_query("show tables from grant_test");
 # These should fail
@@ -195,6 +196,20 @@ user_query("insert into mysql.user (host,user) values ('error','$opt_user',0)",1
 safe_query("revoke ALL PRIVILEGES on $opt_database.* from $user");
 safe_query("select * from mysql.user where user = '$opt_user'");
 safe_query("select * from mysql.db where user = '$opt_user'");
+
+# Test multi-updates
+safe_query("grant CREATE,UPDATE,DROP on $opt_database.* to $user");
+user_connect(0);
+user_query("create table $opt_database.test2 (a int not null)");
+user_query("update test,test2 SET test.a=1 where 1");
+user_query("update test,test2 SET test.a=test2.a where 1",1);
+safe_query("grant SELECT on $opt_database.* to $user");
+user_connect(0);
+user_query("update test,test2 SET test.a=test2.a where test2.a=test.a");
+user_query("drop table $opt_database.test2");
+
+# Revoke database privileges
+safe_query("revoke ALL PRIVILEGES on $opt_database.* from $user");
 user_connect(1);
 
 #
@@ -216,11 +231,18 @@ user_query("insert into $opt_database.test values (8,0)");
 user_query("update $opt_database.test set b=1",1);
 safe_query("grant update on $opt_database.test to $user");
 user_query("update $opt_database.test set b=2");
+
+user_query("update $opt_database.test,test2 SET test.b=3",1);
+safe_query("grant select on $opt_database.test2 to $user");
+user_query("update $opt_database.test,test2 SET test.b=3");
+safe_query("revoke select on $opt_database.test2 from $user");
+
 user_query("delete from $opt_database.test",1);
 safe_query("grant delete on $opt_database.test to $user");
 user_query("delete from $opt_database.test where a=1",1);
 user_query("update $opt_database.test set b=3 where b=1",1);
 user_query("update $opt_database.test set b=b+1",1);
+user_query("update $opt_database.test,test2 SET test.a=test2.a",1);
 
 #
 # Test global SELECT privilege combined with table level privileges
@@ -230,6 +252,8 @@ safe_query("grant SELECT on *.* to $user");
 user_connect(0);
 user_query("update $opt_database.test set b=b+1");
 user_query("update $opt_database.test set b=b+1 where a > 0");
+user_query("update $opt_database.test,test2 SET test.a=test2.a");
+user_query("update $opt_database.test,test2 SET test2.a=test.a",1);
 safe_query("revoke SELECT on *.* from $user");
 safe_query("grant SELECT on $opt_database.* to $user");
 user_connect(0);
@@ -252,6 +276,9 @@ user_query("delete from $opt_database.test where a=1");
 user_query("update $opt_database.test set b=2 where b=1");
 user_query("update $opt_database.test set b=b+1");
 user_query("select count(*) from test");
+user_query("update test,test2 SET test.b=4",1);
+user_query("update test,test2 SET test2.a=test.a",1);
+user_query("update test,test2 SET test.a=test2.a",1);
 
 user_query("create table $opt_database.test3 (a int)",1);
 user_query("alter table $opt_database.test2 add c int",1);
@@ -270,10 +297,27 @@ user_query("select count(*) from test2,test",1);
 user_query("select count(*) from test,test2",1);
 user_query("replace into test2 SELECT a from test",1);
 safe_query("grant update on $opt_database.test2 to $user");
+user_query("update test,test2 SET test2.a=test.a");
+user_query("update test,test2 SET test.b=test2.a where 0",1);
+user_query("update test,test2 SET test.a=2 where test2.a>100",1);
+user_query("update test,test2 SET test.a=test2.a",1);
 user_query("replace into test2 SELECT a,a from test",1);
 safe_query("grant DELETE on $opt_database.test2 to $user");
 user_query("replace into test2 SELECT a,a from test");
 user_query("insert into test (a) SELECT a from test2",1);
+safe_query("grant SELECT on $opt_database.test2 to $user");
+user_query("update test,test2 SET test.b=test2.a where 0");
+user_query("update test,test2 SET test.a=test2.a where test2.a>100");
+
+safe_query("revoke UPDATE on $opt_database.test2 from $user");
+safe_query("grant UPDATE (c) on $opt_database.test2 to $user");
+user_query("update test,test2 SET test.b=test2.a where 0");
+user_query("update test,test2 SET test.a=test2.a where test2.a>100");
+user_query("update test,test2 SET test2.a=test2.a where test2.a>100",1);
+user_query("update test,test2 SET test2.c=test2.a where test2.a>100");
+
+safe_query("revoke SELECT,UPDATE on $opt_database.test2 from $user");
+safe_query("grant UPDATE on $opt_database.test2 to $user");
 
 user_query("drop table $opt_database.test2",1);
 user_query("grant select on $opt_database.test2 to $user with grant option",1);
@@ -315,9 +359,13 @@ user_query("select count(a) from test",1);
 # Test some grants on column level
 #
 
+safe_query("grant create,update on $opt_database.test2 to $user");
+user_query("create table $opt_database.test2 (a int not null)");
 user_query("delete from $opt_database.test where a=2",1);
 user_query("delete from $opt_database.test where A=2",1);
 user_query("update test set b=5 where b>0",1);
+user_query("update test,test2 SET test.b=5 where b>0",1);
+
 safe_query("grant update(b),delete on $opt_database.test to $user");
 safe_query("revoke update(a) on $opt_database.test from $user",1);
 user_query("delete from $opt_database.test where a=2",1);
@@ -327,12 +375,18 @@ user_query("delete from $opt_database.test where a=2");
 user_query("delete from $opt_database.test where A=2");
 user_query("update test set b=5 where b>0");
 user_query("update test set a=11 where b>5",1);
+user_query("update test,test2 SET test.b=5 where b>0");
+user_query("update test,test2 SET test.a=11 where b>0",1);
+user_query("update test,test2 SET test.b=test2.a where b>0",1);
+user_query("update test,test2 SET test.b=11 where test2.a>0",1);
 user_query("select a,A from test");
 
 safe_query("select $tables_cols from mysql.tables_priv");
 safe_query("revoke ALL PRIVILEGES on $opt_database.test from $user");
 safe_query("select $tables_cols from mysql.tables_priv");
 safe_query("revoke GRANT OPTION on $opt_database.test from $user",1);
+safe_query("drop table $opt_database.test2");
+safe_query("revoke create,update on $opt_database.test2 from $user");
 
 #
 # Test grants on database level
@@ -412,7 +466,7 @@ safe_query("select $columns_cols from mysql.columns_priv where user = '$opt_user
 
 safe_query("revoke ALL PRIVILEGES on $opt_database.test from $user");
 user_query("select count(a) from test",1);
-user_query("select * from mysql.user",1);
+user_query("select * from mysql.user order by hostname",1);
 safe_query("select * from mysql.db where user = '$opt_user'");
 safe_query("select $tables_cols from mysql.tables_priv where user = '$opt_user'");
 safe_query("select $columns_cols from mysql.columns_priv where user = '$opt_user'");
@@ -625,7 +679,7 @@ sub user_query
   {
     if (!defined($ignore_error))
     {
-      die "The above should not have failed!";
+      die "Query '$query' should not have failed!";
     }
   }
   elsif (defined($ignore_error) && $ignore_error == 1)
@@ -649,7 +703,7 @@ sub do_query
   if (!$sth->execute)
   {
     $fatal_error= ($DBI::errstr =~ /parse error/);
-    if (!$ignore_error || $opt_verbose || $fatal_error)
+    if (!$ignore_error || ($opt_verbose && $ignore_error != 3) || $fatal_error)
     {
       print "Error in execute: $DBI::errstr\n";
     }