bk commit into 6.0 tree (mhansson:1.2627) BUG#35600

[mhansson]

Below is the list of changes that have just been committed into a local
6.0 repository of mhansson.  When mhansson 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, 2008-04-09 17:30:27+02:00, mhansson@riffraff.(none) +6 -0
  Bug#35600: Security breach via view, I_S table and prepared 
  statement/stored procedure
  
  Fix milestone.
  
  This is a partial fix to the bug. While clearly moving in 
  the right direction, (as acknowledged by Kristofer P.)
  the fix has exposed a number of other bugs, and is already
  getting quite complex.
  
  This commit is made in the runtime tree in order to keep
  up-to-date with the latest changes in check_table_access
  which are not yet pushed to main.

  mysql-test/r/bug35600.result@stripped, 2008-04-09 17:30:18+02:00, mhansson@riffraff.(none)
+43 -0
    Bug#35600: Correct test result

  mysql-test/r/bug35600.result@stripped, 2008-04-09 17:30:18+02:00, mhansson@riffraff.(none) +0
-0

  mysql-test/t/bug35600.test@stripped, 2008-04-09 17:30:24+02:00, mhansson@riffraff.(none) +67
-0
    Bug#35600: Test case.

  mysql-test/t/bug35600.test@stripped, 2008-04-09 17:30:24+02:00, mhansson@riffraff.(none) +0
-0

  mysql-test/t/show_create_view.test@stripped, 2008-04-09 17:30:24+02:00,
mhansson@riffraff.(none) +28 -0
    Bug#35600: Test case. Currently the patch causes a 
    regression by exposing another bug. This test case catches 
    that regression.
    
    A view created with the TEMPTABLE algorithm would always 
    pass through access checking with no questions asked. 
    
    The test case has been distilled out of 
    information_schema_db.test.

  mysql-test/t/show_create_view.test@stripped, 2008-04-09 17:30:24+02:00,
mhansson@riffraff.(none) +0 -0

  sql/sql_acl.cc@stripped, 2008-04-09 17:30:18+02:00, mhansson@riffraff.(none) +4 -4
    Bug#35600: fix inside check_grant
    
    - TABLE_LIST::is_derived_table() is used instead of the 
      unreliable field TABLE_LIST::derived
    
    - We substitute TABLE_LIST::get_database_name() and 
      TABLE_LIST::get_object_name() for TABLE_LIST::db and
      TABLE_LIST::table_name, respectively, since the latter
      two are not set for views.
    

  sql/sql_parse.cc@stripped, 2008-04-09 17:30:18+02:00, mhansson@riffraff.(none) +12 -10
    Bug#35600:
    
    Fixing a regression introduced bug Bug#27145: 
    want_access would remove SELECT_ACL if any table
    in <tables> has TABLE_LIST::schema_table != null

  sql/table.h@stripped, 2008-04-09 17:30:18+02:00, mhansson@riffraff.(none) +59 -0
    Bug#35600:
    
    The field TABLE_LIST::derived was commented to reflect
    its use. Failure to understand this field is what caused
    the bug.
    
    Added predicates is_view() and is_derived_table() that
    will have definite authority to decide whether the 
    TABLE_LIST is a view or derived table, respectively.
    
    Added methods that will always give correct SQL object
    and database name regardless of whether the 
    object is a view or a table.

diff -Nrup a/mysql-test/r/bug35600.result b/mysql-test/r/bug35600.result
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/r/bug35600.result	2008-04-09 17:30:18 +02:00
@@ -0,0 +1,43 @@
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+USE mysqltest1;
+CREATE VIEW v1 AS SELECT * FROM information_schema.tables LIMIT 1;
+CREATE VIEW v2 AS SELECT * FROM information_schema.tables LIMIT 1;
+CREATE VIEW v3 AS SELECT * FROM information_schema.tables LIMIT 1;
+CREATE TABLE t1( a INT );
+CREATE ALGORITHM = MERGE     VIEW v_merge     AS SELECT * FROM t1;
+CREATE ALGORITHM = TEMPTABLE VIEW v_temptable AS SELECT * FROM t1;
+CREATE ALGORITHM = UNDEFINED VIEW v_undefined AS SELECT * FROM t1;
+CREATE                       VIEW v_none      AS SELECT * FROM t1;
+PREPARE stmt1 FROM "SELECT * FROM mysqltest1.v1";
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v1'
+GRANT SELECT ON mysqltest1.* to mysqluser1@localhost;
+PREPARE stmt_v1 FROM "SELECT * FROM mysqltest1.v1";
+PREPARE stmt_v2 FROM "SELECT * FROM mysqltest1.v2";
+PREPARE stmt_v3 FROM "SELECT * FROM mysqltest1.v3";
+PREPARE stmt_merge     FROM "SELECT * FROM mysqltest1.v_merge";
+PREPARE stmt_temptable FROM "SELECT * FROM mysqltest1.v_temptable";
+PREPARE stmt_undefined FROM "SELECT * FROM mysqltest1.v_undefined";
+PREPARE stmt_none      FROM "SELECT * FROM mysqltest1.v_none";
+EXECUTE stmt_v2;
+TABLE_CATALOG	TABLE_SCHEMA	TABLE_NAME	TABLE_TYPE	ENGINE	VERSION	ROW_FORMAT	TABLE_ROWS	AVG_ROW_LENGTH	DATA_LENGTH	MAX_DATA_LENGTH	INDEX_LENGTH	DATA_FREE	AUTO_INCREMENT	CREATE_TIME	UPDATE_TIME	CHECK_TIME	TABLE_COLLATION	CHECKSUM	CREATE_OPTIONS	TABLE_COMMENT
+NULL	information_schema	CHARACTER_SETS	SYSTEM
VIEW	MEMORY	10	Fixed	NULL	766	0	1045590	0	0	NULL	NULL	NULL	NULL	utf8_general_ci	NULL	max_rows=1368	
+REVOKE SELECT ON mysqltest1.* FROM mysqluser1@localhost;
+EXECUTE stmt_v1;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v1'
+EXECUTE stmt_v2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2'
+EXECUTE stmt_v3;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v3'
+EXECUTE stmt_undefined;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table
'v_undefined'
+EXECUTE stmt_merge;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v_merge'
+EXECUTE stmt_temptable;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table
'v_temptable'
+EXECUTE stmt_none;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v_none'
+DROP USER mysqluser1@localhost;
+DROP VIEW v1, v2, v3, v_merge, v_temptable, v_undefined, v_none;
+DROP TABLE t1;
+DROP DATABASE mysqltest1;
diff -Nrup a/mysql-test/t/bug35600.test b/mysql-test/t/bug35600.test
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/t/bug35600.test	2008-04-09 17:30:24 +02:00
@@ -0,0 +1,67 @@
+# to do: name connections connection1 etc
+
+
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+USE mysqltest1;
+
+CREATE VIEW v1 AS SELECT * FROM information_schema.tables LIMIT 1;
+CREATE VIEW v2 AS SELECT * FROM information_schema.tables LIMIT 1;
+CREATE VIEW v3 AS SELECT * FROM information_schema.tables LIMIT 1;
+
+CREATE TABLE t1( a INT );
+
+CREATE ALGORITHM = MERGE     VIEW v_merge     AS SELECT * FROM t1;
+CREATE ALGORITHM = TEMPTABLE VIEW v_temptable AS SELECT * FROM t1;
+CREATE ALGORITHM = UNDEFINED VIEW v_undefined AS SELECT * FROM t1;
+CREATE                       VIEW v_none      AS SELECT * FROM t1;
+
+--connect (mysqluser1, localhost, mysqluser1, , test)
+# We try prepare on only one of the views, as the following statement will
+# alter the view object.
+--error ER_TABLEACCESS_DENIED_ERROR
+PREPARE stmt1 FROM "SELECT * FROM mysqltest1.v1";
+
+--connection default
+GRANT SELECT ON mysqltest1.* to mysqluser1@localhost;
+#GRANT SELECT ON mysqltest1.v2 to mysqluser1@localhost;
+
+--connection mysqluser1
+PREPARE stmt_v1 FROM "SELECT * FROM mysqltest1.v1";
+PREPARE stmt_v2 FROM "SELECT * FROM mysqltest1.v2";
+PREPARE stmt_v3 FROM "SELECT * FROM mysqltest1.v3";
+
+PREPARE stmt_merge     FROM "SELECT * FROM mysqltest1.v_merge";
+PREPARE stmt_temptable FROM "SELECT * FROM mysqltest1.v_temptable";
+PREPARE stmt_undefined FROM "SELECT * FROM mysqltest1.v_undefined";
+PREPARE stmt_none      FROM "SELECT * FROM mysqltest1.v_none";
+
+EXECUTE stmt_v2;
+
+--connection default
+REVOKE SELECT ON mysqltest1.* FROM mysqluser1@localhost;
+
+--connection mysqluser1
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_v1;
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_v2;
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_v3;
+# This worked previously as well
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_undefined;
+# This worked previously as well
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_merge;
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_temptable;
+# This worked previously as well
+--error ER_TABLEACCESS_DENIED_ERROR
+EXECUTE stmt_none;
+
+--connection default
+DROP USER mysqluser1@localhost;
+DROP VIEW v1, v2, v3, v_merge, v_temptable, v_undefined, v_none;
+DROP TABLE t1;
+DROP DATABASE mysqltest1;
diff -Nrup a/mysql-test/t/show_create_view.test b/mysql-test/t/show_create_view.test
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/t/show_create_view.test	2008-04-09 17:30:24 +02:00
@@ -0,0 +1,28 @@
+
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+
+USE mysqltest1;
+
+CREATE TABLE t2 ( a INT, b INT );
+CREATE ALGORITHM = MERGE VIEW v4 AS SELECT a, b FROM t2;
+CREATE ALGORITHM = MERGE VIEW v5 AS SELECT a, b FROM t2;
+GRANT SELECT( a ), SELECT( b ) ON v4 TO mysqluser1@localhost;
+GRANT INSERT( a ), INSERT( b ) ON v5 TO mysqluser1@localhost;
+
+--connect (connection1, localhost, mysqluser1, , test)
+
+CREATE ALGORITHM = MERGE VIEW v4 AS SELECT a, b FROM mysqltest1.v4;
+CREATE ALGORITHM = MERGE VIEW v5 AS SELECT a, b FROM mysqltest1.v5;
+
+--connection default
+
+REVOKE SELECT( a ) ON v4 FROM mysqluser1@localhost;
+REVOKE INSERT( a ) ON v5 FROM mysqluser1@localhost;
+
+--connection connection1
+
+--error ER_VIEW_NO_EXPLAIN
+SHOW CREATE VIEW v4;
+--error ER_VIEW_NO_EXPLAIN
+SHOW CREATE VIEW v5;
diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
--- a/sql/sql_acl.cc	2008-04-08 21:16:09 +02:00
+++ b/sql/sql_acl.cc	2008-04-09 17:30:18 +02:00
@@ -3920,7 +3920,7 @@ bool check_grant(THD *thd, ulong want_ac
       continue;                                 // ok
 
     if (!(~table->grant.privilege & want_access) || 
-        table->derived || table->schema_table)
+        table->is_derived_table() || table->schema_table)
     {
       /*
         It is subquery in the FROM clause. VIEW set table->derived after
@@ -3938,8 +3938,8 @@ bool check_grant(THD *thd, ulong want_ac
       continue;
     }
     if (!(grant_table= table_hash_search(sctx->host, sctx->ip,
-                                         table->db, sctx->priv_user,
-                                         table->table_name,0)))
+                                         table->get_database_name(),
sctx->priv_user,
+                                         table->get_object_name(), FALSE)))
     {
       want_access &= ~table->grant.privilege;
       goto err;					// No grants
@@ -3980,7 +3980,7 @@ err:
              command,
              sctx->priv_user,
              sctx->host_or_ip,
-             table ? table->table_name : "unknown");
+             table ? table->get_object_name() : "unknown");
   }
   DBUG_RETURN(TRUE);
 }
diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
--- a/sql/sql_parse.cc	2008-04-09 11:06:14 +02:00
+++ b/sql/sql_parse.cc	2008-04-09 17:30:18 +02:00
@@ -5187,7 +5187,7 @@ static bool check_show_access(THD *thd, 
 */
 
 bool
-check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
+check_table_access(THD *thd, const ulong want_access,TABLE_LIST *tables,
 		   bool no_errors, bool any_combination_of_privileges_will_do,
                    uint number)
 {
@@ -5203,6 +5203,7 @@ check_table_access(THD *thd, ulong want_
   for (; i < number && tables != first_not_own_table && tables;
        tables= tables->next_global, i++)
   {
+    ulong requirements= want_access; 
     if (tables->security_ctx)
       sctx= tables->security_ctx;
     else
@@ -5210,13 +5211,13 @@ check_table_access(THD *thd, ulong want_
 
     /*
       Always allow SELECT on schema tables. This is done by removing the 
-      required SELECT_ACL privilege in the want_access parameter.
+      required SELECT_ACL privilege in the requirements parameter.
       Disallow any other DDL or DML operation on any schema table.
     */
     if (tables->schema_table)
     {
-      want_access &= ~SELECT_ACL;
-      if (want_access & DB_ACLS)
+      requirements &= ~SELECT_ACL;
+      if (requirements & DB_ACLS)
       {
         if (!no_errors)
           my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
@@ -5229,7 +5230,7 @@ check_table_access(THD *thd, ulong want_
        Register access for view underlying table.
        Remove SHOW_VIEW_ACL, because it will be checked during making view
      */
-    tables->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
+    tables->grant.orig_want_privilege= (requirements & ~SHOW_VIEW_ACL);
 
     if (tables->schema_table_reformed)
     {
@@ -5238,14 +5239,15 @@ check_table_access(THD *thd, ulong want_
       continue;
     }
 
-    if (tables->derived ||
+    if (tables->is_derived_table() ||
         (tables->table && tables->table->s &&
(int)tables->table->s->tmp_table))
       continue;
     thd->security_ctx= sctx;
-    if ((sctx->master_access & want_access) ==
-        want_access && thd->db)
-      tables->grant.privilege= want_access;
-    else if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
+    if ((sctx->master_access & requirements) ==
+        requirements && thd->db)
+      tables->grant.privilege= requirements;
+    else if (check_access(thd,requirements,tables->get_database_name(), 
+                          &tables->grant.privilege,
                           0, no_errors, 0))
       goto deny;
   }
diff -Nrup a/sql/table.h b/sql/table.h
--- a/sql/table.h	2008-03-14 23:20:08 +01:00
+++ b/sql/table.h	2008-04-09 17:30:18 +02:00
@@ -70,6 +70,12 @@ typedef struct st_grant_info
 {
   GRANT_TABLE *grant_table;
   uint version;
+  /**
+     @brief An ACL represented as a bitmap represented as a ulong. The
+     privileges are defined as macros.
+
+     @see sql_acl.h (the macros whose names end in _ACL)
+  */
   ulong privilege;
   ulong want_privilege;
   /*
@@ -1016,6 +1022,25 @@ struct TABLE_LIST
     can see this lists can't be merged)
   */
   TABLE_LIST	*correspondent_table;
+  /**
+     @brief Normally, this is non-null for derived tables only.
+     @details This field is set to non-null for 
+
+     - Derived tables, I.e. queries in the @c FROM clause. In this case it
+       points to the SELECT_LEX_UNIT representing the derived table. E.g. for
+       a query
+
+       @verbatim SELECT * FROM (SELECT a FROM t1) b @endverbatim
+
+       For the @c TABLE_LIST representing the derived table @c b, @c derived
+       points to the @c SELECT_LEX_UNIT representing the whole query within
+       parenteses.
+
+     - Views. This is set for views with @verbatin ALGORITHM = TEMPTABLE
+       @endverbatim in the function @c mysql_make_view().
+
+     Note that in views, a subquery in the @c FROM clause is not allowed.
+   */
   st_select_lex_unit *derived;		/* SELECT_LEX_UNIT of derived table */
   ST_SCHEMA_TABLE *schema_table;        /* Information_schema table */
   st_select_lex	*schema_select_lex;
@@ -1237,6 +1262,40 @@ struct TABLE_LIST
     procedure.
   */
   void reinit_before_use(THD *thd);
+
+  /**
+     @brief If this is set, this @c TABLE_LIST represents a view and nothing
+     else.
+   */
+  bool is_view() { return view != NULL; }
+
+  /**
+     @brief If true, this @c TABLE_LIST represents a derived table and nothing
+     else.
+
+     @note By derived table, we mean the result of a nested query in the @c
+     FROM clause of its upper query.
+   */
+  bool is_derived_table() { return derived != NULL && view == NULL; }
+
+  /**
+     @brief Returns the name of the SQL object that this @c TABLE_LIST
+     represents.  
+
+     @details The object name would be the unqualified table name or view name
+     for a table or view, respectively.
+
+     @note Because of legacy reasons, this is a @c char* and not a String.
+   */
+  char *get_object_name() { return is_view() ? view_name.str : table_name; }
+
+  /**
+     @brief Returns the database name (or schema name in SQL:2003 terminology)
+     that this database belongs to.
+
+     @note Because of legacy reasons, this is a @c char* and not a String.
+   */
+  char *get_database_name() { return is_view() ? view_db.str : db; }
   Item_subselect *containing_subselect();
 
   /*