From: Date: April 9 2008 5:31pm Subject: bk commit into 6.0 tree (mhansson:1.2627) BUG#35600 List-Archive: http://lists.mysql.com/commits/45145 X-Bug: 35600 Message-Id: <200804091531.m39FVVUs018175@riffraff> 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 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(); /*