From: Date: November 27 2009 12:00am Subject: bzr commit into mysql-6.0-bugteam tree (mhansson:2656) Bug#36086 List-Archive: http://lists.mysql.com/commits/47062 X-Bug: 36086 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0184581160==" --===============0184581160== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///data0/martin/bzr/6.0bt-bug36086/ ------------------------------------------------------------ revno: 2656 revision-id: mhansson@stripped parent: kpettersson@stripped committer: Martin Hansson branch nick: 6.0bt-bug36086 timestamp: Mon 2008-05-26 20:52:43 +0200 message: Bug#36086: SELECT * from views don't check column grants A "SELECT *" against an ALGORITHM=TEMPTABLE wrongfully treated a view as an anonymous derived table, i.e. access checking was skipped. Fixed by introducing a predicate to tell the difference between named and anonymous derived tables. modified: mysql-test/r/view_grant.result sp1f-view_grant.result-20050404194355-hbbr5ud3thpo5tn65q6eyecswq5mdhwk mysql-test/t/view_grant.test sp1f-view_grant.test-20050404194355-y5ik7soywcms7xriyzo72dooviahc7cx sql/sql_acl.cc sp1f-sql_acl.cc-19700101030959-c4hku3uqxzujthqnndeprbrhamqy6a4i sql/sql_derived.cc sp1f-sql_derived.cc-20020326130604-4qz6ovo2xa6w5eslbmcx76agmnyyvsfh sql/table.h sp1f-table.h-19700101030959-dv72bajftxj5fbdjuajquappanuv2ija per-file comments: mysql-test/r/view_grant.result Bug36086: Test result mysql-test/t/view_grant.test Bug36086: Test case sql/sql_acl.cc Bug#36086: Updated comment. This function was previously not called for views. sql/sql_derived.cc Bug#36086: - changed comment to Doxygen standard and added content. - The fix. A view is implemented as a named derived table (or projected onto a temporary table) and it should not automatically be assumed that the user has fulfilled the SELECT privileges. However, for anonymous derived tables no privileges are required. sql/table.h Bug#36086: - Commented the GRANT_INFO structure and members. - Added predicate to be able to explicitly tell when a TABLE_LIST represents an anonymous derived table in the from clause. Currently, this is the only case that mysql_derived_prepare must handle. --===============0184581160== MIME-Version: 1.0 Content-Type: text/text/x-diff; charset="us-ascii"; name="patch-2656.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline === modified file 'mysql-test/r/view_grant.result' --- a/mysql-test/r/view_grant.result 2008-03-22 08:02:24 +0000 +++ b/mysql-test/r/view_grant.result 2008-05-26 18:52:43 +0000 @@ -948,3 +948,44 @@ DROP VIEW v1; DROP TABLE t1; End of 5.1 tests. +CREATE USER mysqluser1@localhost; +CREATE DATABASE mysqltest1; +USE mysqltest1; +CREATE TABLE t1 ( a INT, b INT ); +CREATE TABLE t2 ( a INT, b INT ); +CREATE VIEW v1 AS SELECT 1 AS a, 2 AS b; +CREATE VIEW v2 AS SELECT 1 AS a, 2 AS b; +GRANT SELECT( a ), SELECT( b ) ON t1 TO mysqluser1@localhost; +GRANT INSERT( a ), INSERT( b ) ON t2 TO mysqluser1@localhost; +GRANT SELECT( a ), SELECT( b ) ON v1 TO mysqluser1@localhost; +GRANT INSERT( a ), INSERT( b ) ON v2 TO mysqluser1@localhost; +SELECT a, b FROM v1; +a b +1 2 +SELECT * FROM v1; +a b +1 2 +SELECT a, b FROM v2; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2' +SELECT * FROM v2; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2' +REVOKE SELECT( a ) ON t1 FROM mysqluser1@localhost; +REVOKE INSERT( a ) ON t2 FROM mysqluser1@localhost; +REVOKE SELECT( a ) ON v1 FROM mysqluser1@localhost; +REVOKE INSERT( a ) ON v2 FROM mysqluser1@localhost; +"Worked prior to fix." +SELECT a, b FROM mysqltest1.v2; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2' +"Worked prior to fix." +SELECT * FROM mysqltest1.v2; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2' +"Worked prior to fix." +SELECT a, b FROM mysqltest1.v1; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 'v1' +SELECT * FROM mysqltest1.v1; +ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 'v1' +DROP TABLE t1, t2; +DROP VIEW v1, v2; +DROP DATABASE mysqltest1; +DROP USER mysqluser1@localhost; +USE test; === modified file 'mysql-test/t/view_grant.test' --- a/mysql-test/t/view_grant.test 2008-03-04 17:35:42 +0000 +++ b/mysql-test/t/view_grant.test 2008-05-26 18:52:43 +0000 @@ -1216,3 +1216,64 @@ DROP TABLE t1; --echo End of 5.1 tests. + +# +# SELECT * from views don't check column grants +# +CREATE USER mysqluser1@localhost; +CREATE DATABASE mysqltest1; + +USE mysqltest1; + +CREATE TABLE t1 ( a INT, b INT ); +CREATE TABLE t2 ( a INT, b INT ); + +CREATE VIEW v1 AS SELECT 1 AS a, 2 AS b; +CREATE VIEW v2 AS SELECT 1 AS a, 2 AS b; + +GRANT SELECT( a ), SELECT( b ) ON t1 TO mysqluser1@localhost; +GRANT INSERT( a ), INSERT( b ) ON t2 TO mysqluser1@localhost; + +GRANT SELECT( a ), SELECT( b ) ON v1 TO mysqluser1@localhost; +GRANT INSERT( a ), INSERT( b ) ON v2 TO mysqluser1@localhost; + +--connect (connection1, localhost, mysqluser1, , mysqltest1) + +SELECT a, b FROM v1; +SELECT * FROM v1; +--error ER_TABLEACCESS_DENIED_ERROR +SELECT a, b FROM v2; +--error ER_TABLEACCESS_DENIED_ERROR +SELECT * FROM v2; + +--connection default + +REVOKE SELECT( a ) ON t1 FROM mysqluser1@localhost; +REVOKE INSERT( a ) ON t2 FROM mysqluser1@localhost; + +REVOKE SELECT( a ) ON v1 FROM mysqluser1@localhost; +REVOKE INSERT( a ) ON v2 FROM mysqluser1@localhost; + +--connection connection1 + +--echo "Worked prior to fix." +--error ER_TABLEACCESS_DENIED_ERROR +SELECT a, b FROM mysqltest1.v2; +--echo "Worked prior to fix." +--error ER_TABLEACCESS_DENIED_ERROR +SELECT * FROM mysqltest1.v2; +--echo "Worked prior to fix." +--error ER_COLUMNACCESS_DENIED_ERROR +SELECT a, b FROM mysqltest1.v1; +--error ER_COLUMNACCESS_DENIED_ERROR +SELECT * FROM mysqltest1.v1; + +--disconnect connection1 +--connection default + +DROP TABLE t1, t2; +DROP VIEW v1, v2; +DROP DATABASE mysqltest1; +DROP USER mysqluser1@localhost; + +USE test; === modified file 'sql/sql_acl.cc' --- a/sql/sql_acl.cc 2008-05-08 16:01:15 +0000 +++ b/sql/sql_acl.cc 2008-05-26 18:52:43 +0000 @@ -4224,7 +4224,7 @@ @retval 1 Falure @details This function walks over the columns of a table reference The columns may originate from different tables, depending on the kind of - table reference, e.g. join. + table reference, e.g. join, view. For each table it will retrieve the grant information and will use it to check the required access privileges for the fields requested from it. */ === modified file 'sql/sql_derived.cc' --- a/sql/sql_derived.cc 2007-12-12 15:44:48 +0000 +++ b/sql/sql_derived.cc 2008-05-26 18:52:43 +0000 @@ -73,29 +73,59 @@ } -/* - Create temporary table structure (but do not fill it) - - SYNOPSIS - mysql_derived_prepare() - thd Thread handle - lex LEX for this thread - orig_table_list TABLE_LIST for the upper SELECT - - IMPLEMENTATION - Derived table is resolved with temporary table. - - After table creation, the above TABLE_LIST is updated with a new table. - - This function is called before any command containing derived table - is executed. - - Derived tables is stored in thd->derived_tables and freed in - close_thread_tables() - - RETURN - FALSE OK - TRUE Error +/** + @brief Create temporary table structure (but do not fill it). + + @param thd Thread handle + @param lex LEX for this thread + @param orig_table_list TABLE_LIST for the upper SELECT + + @details + + This function is called before any command containing derived tables is + executed. Currently the function is used for derived tables, i.e. + + - Anonymous derived tables, or + - Named derived tables (aka views) with the @c TEMPTABLE algorithm. + + The table reference, contained in orig_table_list, is updated with the + fields of a new temporary table. + + Derived tables are stored in thd->derived_tables and closed by + close_thread_tables(). + + This function is part of the procedure that starts in + open_and_lock_tables(), a procedure that - among many other things - + introduces new table and table reference objects (to represent derived + tables) that don't exist in the privilege database. This means that normal + privilege checking cannot handle them. Hence this function does some extra + tricks in order to bypass normal privilege checking, by exploiting the fact + that the current state of privilege verification is attached on the + relevant TABLE and TABLE_REF objects. + + For table references, the current state of accrued access is stored inside + TABLE_LIST::grant. Hence this function must update the state of fulfilled + privileges for the new TABLE_LIST, an operation which is normally performed + exclusively by the table and database access checking functions, + check_access() and check_grant(), respectively. This modifification is done + differently depending on whether a view or an anonymous derived table is + modified. + + - If an anonymous derived table is referenced, the @c SELECT privilege is + set as fulfilled by the user. + + - If a view is referenced and the table reference is queried against + directly (see TABLE_LIST::referencing_view), the state of privilege + checking (GRANT_INFO struct) is copied as-is to the temporary table. + + This function implements a signature called "derived table processor", and + is passed as a function pointer to mysql_handle_derived(). + + @see mysql_handle_derived(), mysql_derived_filling(), GRANT_INFO + + @return + false OK + true Error */ bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *orig_table_list) @@ -185,7 +215,7 @@ #ifndef NO_EMBEDDED_ACCESS_CHECKS if (orig_table_list->referencing_view) table->grant= orig_table_list->grant; - else + else if (orig_table_list->is_anonymous_derived_table()) table->grant.privilege= SELECT_ACL; #endif orig_table_list->db= (char *)""; === modified file 'sql/table.h' --- a/sql/table.h 2008-05-08 13:01:30 +0000 +++ b/sql/table.h 2008-05-26 18:52:43 +0000 @@ -66,13 +66,54 @@ table_map used, depend_map; } ORDER; +/** + @brief The current state of the privilege checking process for the current + user, SQL statement and SQL object. + + @details The privilege checking process is divided into phases depending on + the level of the privilege to be checked and the type of object to be + accessed. Due to the distributed nature of privilege checking, it is + necessary to keep track of the state of the process. This information is + stored in privilege, want_privilege, and orig_want_privilege. + + A GRANT_INFO also serves as a local repository of the privilege + database. Relevant members are grant_table and version. + */ typedef struct st_grant_info { + /** + @brief A copy of the privilege information regarding the current host, + database, object and user. + + @details The version of this copy is found in GRANT_INFO::version. + */ GRANT_TABLE *grant_table; + /** + @brief Used for cache invalidation when caching privile information. + + @details The privilege information is stored on disk, with dedicated + caches residing in memory: table-level and column-level privileges, + respectively, have their own dedicated cache. + */ uint version; + /** + @brief The set of privileges that the current user has fulfilled for a + certain host, database, and object. + + @details This field is continually updated throughout the access checking + process. In each step the "wanted privilege" is checked against the + fulfilled privileges. When/if the intersection of these sets is empty, + access is granted. + + The set is implemented as a bitmap, with the bits defined in sql_acl.h. + */ ulong privilege; + /** + @brief the set of privileges that the current user needs to fulfil in + order to carry out the requested operation. + */ ulong want_privilege; - /* + /** Stores the requested access acl of top level tables list. Is used to check access rights to the underlying tables of a view. */ @@ -1267,6 +1308,15 @@ child_def_version= ~0UL; } + /** + @brief If true, this @c TABLE_LIST represents an @i anonymous @i derived + @i table in the @c FROM clause. + + @note An anonymous derived table is the result of a subquery and cannot + be a view. + */ + bool is_anonymous_derived_table() { return derived != NULL && view == NULL; } + private: bool prep_check_option(THD *thd, uint8 check_opt_type); bool prep_where(THD *thd, Item **conds, bool no_where_clause); --===============0184581160==--