List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:August 21 2009 9:57am
Subject:bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936) Bug#35996
View as plain text  
#At file:///data0/martin/bzr/bug35996/5.1bt-gca-commmit/ based on revid:davi.arnaut@stripped

 2936 Martin Hansson	2009-08-21
      Bug#35996: Security Breach In Smashed TEMPTABLE Views
      
      There were no errors displayed when issuing a SHOW CREATE VIEW for views that
      reference base tables for which the user did not have sufficient privileges to
      see the table structure. If the view referenced a view with the same lack of
      privileges, however, an error was raised correctly. 
      
      This came about because the 'access denied' error message was first issued
      during normal access checking for the referenced base table, then converted
      into a generic 'view invalid' message for the referencing view in order to
      hide details of the table structure which were otherwise visible in the error
      message.
      
      Later still, all 'view invalid' errors were cleared and a warning issued
      instead, the rationale being that we should not get errors simply because a
      view referenced a nonexisting object. At this point all information about the
      initial causes of the error condition were lost. 
      
      Fixed by implementing a specialized subclass of Internal_error_handler and
      removing error handling that manipulates error messages.
     @ mysql-test/r/view_grant.result
        Bug#35996: Test result.
     @ mysql-test/t/view_grant.test
        Bug#35996: Test case.
     @ sql/sql_base.cc
        Bug#35996: Partial removal of old style of error handling.
     @ sql/sql_show.cc
        Bug#35996: Implementation of the new Internal_error_handler subclass.

    modified:
      mysql-test/r/view_grant.result
      mysql-test/t/view_grant.test
      sql/sql_base.cc
      sql/sql_show.cc
=== modified file 'mysql-test/r/view_grant.result'
--- a/mysql-test/r/view_grant.result	2009-05-15 12:57:51 +0000
+++ b/mysql-test/r/view_grant.result	2009-08-21 09:57:23 +0000
@@ -1044,3 +1044,57 @@ DROP DATABASE mysqltest1;
 DROP VIEW test.v3;
 DROP USER mysqluser1@localhost;
 USE test;
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+GRANT USAGE, SELECT, CREATE VIEW, SHOW VIEW 
+ON mysqltest2.* TO mysqluser1@localhost;
+USE mysqltest1;
+CREATE TABLE t1( a INT );
+CREATE VIEW v1 AS SELECT 1 AS a;
+GRANT SELECT ON t1 TO mysqluser1@localhost;
+GRANT SELECT ON v1 TO mysqluser1@localhost;
+CREATE VIEW v_t1 AS SELECT * FROM mysqltest1.t1;
+CREATE VIEW v_v1 AS SELECT * FROM mysqltest1.v1;
+REVOKE SELECT ON t1 FROM mysqluser1@localhost;
+REVOKE SELECT ON v1 FROM mysqluser1@localhost;
+SHOW CREATE VIEW v_t1;
+ERROR HY000: EXPLAIN/SHOW can not be issued; lacking privileges for underlying table
+SHOW CREATE VIEW v_v1;
+ERROR HY000: EXPLAIN/SHOW can not be issued; lacking privileges for underlying table
+DROP TABLE t1;
+DROP VIEW v1;
+SHOW CREATE VIEW v_t1;
+View	Create View	character_set_client	collation_connection
+v_t1	CREATE ALGORITHM=UNDEFINED DEFINER=`mysqluser1`@`localhost` SQL SECURITY DEFINER VIEW `v_t1` AS select `mysqltest1`.`t1`.`a` AS `a` from `mysqltest1`.`t1`	latin1	latin1_swedish_ci
+Warnings:
+Warning	1356	View 'mysqltest2.v_t1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
+SHOW CREATE VIEW v_v1;
+View	Create View	character_set_client	collation_connection
+v_v1	CREATE ALGORITHM=UNDEFINED DEFINER=`mysqluser1`@`localhost` SQL SECURITY DEFINER VIEW `v_v1` AS select `v1`.`a` AS `a` from `mysqltest1`.`v1`	latin1	latin1_swedish_ci
+Warnings:
+Warning	1356	View 'mysqltest2.v_v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
+DROP USER mysqluser1@localhost;
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+USE test;
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1() AS a;
+DROP FUNCTION f1;
+SHOW CREATE VIEW v1;
+View	Create View	character_set_client	collation_connection
+v1	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `f1`() AS `a`	latin1	latin1_swedish_ci
+Warnings:
+Warning	1356	View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
+DROP VIEW v1;
+CREATE TABLE t1( a INT );
+CREATE DEFINER = no_such_user@no_such_host VIEW v1 AS SELECT * FROM t1;
+Warnings:
+Note	1449	The user specified as a definer ('no_such_user'@'no_such_host') does not exist
+SHOW CREATE VIEW v1;
+View	Create View	character_set_client	collation_connection
+v1	CREATE ALGORITHM=UNDEFINED DEFINER=`no_such_user`@`no_such_host` SQL SECURITY DEFINER VIEW `v1` AS select `test`.`t1`.`a` AS `a` from `t1`	latin1	latin1_swedish_ci
+Warnings:
+Warning	1356	View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
+DROP TABLE t1;
+DROP VIEW v1;

=== modified file 'mysql-test/t/view_grant.test'
--- a/mysql-test/t/view_grant.test	2009-05-15 12:57:51 +0000
+++ b/mysql-test/t/view_grant.test	2009-08-21 09:57:23 +0000
@@ -1385,3 +1385,60 @@ USE test;
 # Wait till we reached the initial number of concurrent sessions
 --source include/wait_until_count_sessions.inc
 
+#
+# Bug#35996: Security Breach In Smashed TEMPTABLE Views
+#
+-- source include/not_embedded.inc
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+GRANT USAGE, SELECT, CREATE VIEW, SHOW VIEW 
+ON mysqltest2.* TO mysqluser1@localhost;
+
+USE mysqltest1;
+
+CREATE TABLE t1( a INT );
+CREATE VIEW v1 AS SELECT 1 AS a;
+
+GRANT SELECT ON t1 TO mysqluser1@localhost;
+GRANT SELECT ON v1 TO mysqluser1@localhost;
+
+--connect (connection1, localhost, mysqluser1,, mysqltest2)
+CREATE VIEW v_t1 AS SELECT * FROM mysqltest1.t1;
+CREATE VIEW v_v1 AS SELECT * FROM mysqltest1.v1;
+
+--connection default
+REVOKE SELECT ON t1 FROM mysqluser1@localhost;
+REVOKE SELECT ON v1 FROM mysqluser1@localhost;
+
+--connection connection1
+--error ER_VIEW_NO_EXPLAIN
+SHOW CREATE VIEW v_t1;
+--error ER_VIEW_NO_EXPLAIN
+SHOW CREATE VIEW v_v1;
+
+--connection default
+DROP TABLE t1;
+DROP VIEW v1;
+--connection connection1
+SHOW CREATE VIEW v_t1;
+SHOW CREATE VIEW v_v1;
+
+--disconnect connection1
+--connection default
+DROP USER mysqluser1@localhost;
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+USE test;
+
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1() AS a;
+DROP FUNCTION f1;
+SHOW CREATE VIEW v1;
+DROP VIEW v1;
+
+CREATE TABLE t1( a INT );
+CREATE DEFINER = no_such_user@no_such_host VIEW v1 AS SELECT * FROM t1;
+SHOW CREATE VIEW v1;
+DROP TABLE t1;
+DROP VIEW v1;

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2009-05-30 13:32:28 +0000
+++ b/sql/sql_base.cc	2009-08-21 09:57:23 +0000
@@ -7644,7 +7644,12 @@ bool setup_tables_and_check_access(THD *
         check_single_table_access(thd, first_table ? want_access_first :
                                   want_access, leaves_tmp, FALSE))
     {
-      tables->hide_view_error(thd);
+      /* 
+         Unless the Show_create_error_handler is in effect, revert to old
+         method of supressing errors.
+      */
+      if (!thd->get_internal_handler())
+        tables->hide_view_error(thd);
       return TRUE;
     }
     first_table= 0;

=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc	2009-05-15 12:57:51 +0000
+++ b/sql/sql_show.cc	2009-08-21 09:57:23 +0000
@@ -584,6 +584,124 @@ find_files(THD *thd, List<LEX_STRING> *f
 }
 
 
+/**
+   An Internal_error_handler that downgrades SHOW CREATE VIEW errors for
+   certain invalid views to warnings. 
+   
+   This happens in the cases when a view's
+   underlying object (e.g. referenced in its SELECT list) does not exist.
+
+   - For views referencing non-existing functions
+
+   - For views referencing non-existing tables   
+ */
+class Show_create_error_handler : public Internal_error_handler 
+{
+private:
+  
+  /**
+     The purpose of the Show_create_error_handler is to hide details of
+     underlying tables for which we have no privileges behind ER_VIEW_INVALID
+     messages. But this obviously does not apply if we lack privileges on the
+     view itself. Unfortunately the information about for which table privilege
+     checking failed is not available at the point of calling
+     handle_error(). The only way for us to check is by reconstructing the
+     actual error message and use strcmp() to see if it's the same.
+  */
+  char view_access_denied_message[MYSQL_ERRMSG_SIZE];
+  
+  TABLE_LIST *table;
+  bool within_handler;
+
+  /**
+     We record the user and host at construction of this error handler, since
+     the privilege checking procedure will manipulate the security context in
+     thd.
+   */
+  const char *original_user, *original_host;
+
+  bool handle_error_impl(uint sql_errno, const char *message, 
+                         MYSQL_ERROR::enum_warning_level level, THD *thd)
+  {
+    
+    switch (sql_errno) {
+
+    case ER_NO_SUCH_TABLE:
+      /* If an underlying table is missing, just warn. */
+      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, 
+                          ER_VIEW_INVALID,
+                          ER(ER_VIEW_INVALID),
+                          table->get_db_name(),
+                          table->get_table_name());
+      return TRUE;
+      
+    case ER_VIEW_INVALID:
+    case ER_NO_SUCH_USER:
+      /* 
+         If a user or an underlying function does not exist, issue warning
+         only. Unfortunately we have to manipulate error queue, this is
+         because this class was not involved when those errors were issued,
+         and the code issuing them relies on later code to correct the error
+         messages/codes.
+      */
+      mysql_reset_errors(thd, true);
+      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, 
+                          ER_VIEW_INVALID,
+                          ER(ER_VIEW_INVALID),
+                          table->get_db_name(),
+                          table->get_table_name());
+      return TRUE;
+    case ER_TABLEACCESS_DENIED_ERROR:
+      if (!strcmp(view_access_denied_message, message))
+        /* If access to the view itself is not granted, don't interfere. */
+        return FALSE;
+      /* 
+         Missing privilege on underlying table, throw an error. We only care
+         about the current user's privileges, however. check_grant will check
+         the view definer's credentials on underlying tables. During this
+         procedure, thd->security_ctx is that of the definer.
+       */
+      if (!strcmp(original_user, thd->security_ctx->priv_user) &&
+          !strcmp(original_host, thd->security_ctx->host_or_ip))
+        my_error(ER_VIEW_NO_EXPLAIN, MYF(0));
+      return TRUE;
+    }
+    return FALSE;
+  }
+  
+public:
+  explicit Show_create_error_handler(THD *thd, TABLE_LIST *table_list) : 
+    table(table_list), within_handler(FALSE)
+  {
+    Security_context *sctx = test(table->security_ctx) ?
+      table->security_ctx : thd->security_ctx;
+    original_user= sctx->priv_user;
+    original_host= sctx->host_or_ip;
+    my_snprintf(view_access_denied_message, MYSQL_ERRMSG_SIZE,
+                ER(ER_TABLEACCESS_DENIED_ERROR), "SHOW VIEW",
+                sctx->priv_user, sctx->host_or_ip, table->get_table_name());
+  }
+  
+  bool handle_error(uint sql_errno, const char *message, 
+                    MYSQL_ERROR::enum_warning_level level, THD *thd)
+  {
+
+    /* Only view errors are handled. */
+    if (!table->view)
+      return FALSE;
+
+    /* The handler does not handle the errors raised by itself, obviously. */
+    if (within_handler)
+      return FALSE;
+    
+    within_handler= TRUE;
+    bool is_handled= handle_error_impl(sql_errno, message, level, thd);
+    within_handler= FALSE;
+    return is_handled;
+  }
+};
+
+
 bool
 mysqld_show_create(THD *thd, TABLE_LIST *table_list)
 {
@@ -597,27 +715,12 @@ mysqld_show_create(THD *thd, TABLE_LIST 
   /* We want to preserve the tree for views. */
   thd->lex->view_prepare_mode= TRUE;
 
-  /* Only one table for now, but VIEW can involve several tables */
-  if (open_normal_and_derived_tables(thd, table_list, 0))
-  {
-    if (!table_list->view ||
-        thd->is_error() && thd->main_da.sql_errno() != ER_VIEW_INVALID)
-      DBUG_RETURN(TRUE);
-
-    /*
-      Clear all messages with 'error' level status and
-      issue a warning with 'warning' level status in 
-      case of invalid view and last error is ER_VIEW_INVALID
-    */
-    mysql_reset_errors(thd, true);
-    thd->clear_error();
-
-    push_warning_printf(thd,MYSQL_ERROR::WARN_LEVEL_WARN,
-                        ER_VIEW_INVALID,
-                        ER(ER_VIEW_INVALID),
-                        table_list->view_db.str,
-                        table_list->view_name.str);
-  }
+  Show_create_error_handler view_error_suppressor(thd, table_list);
+  thd->push_internal_handler(&view_error_suppressor);
+  bool error= open_normal_and_derived_tables(thd, table_list, 0);
+  thd->pop_internal_handler();
+  if (error && thd->main_da.is_error())
+    DBUG_RETURN(TRUE);
 
   /* TODO: add environment variables show when it become possible */
   if (thd->lex->only_view && !table_list->view)


Attachment: [text/bzr-bundle] bzr/martin.hansson@sun.com-20090821095723-4d39ea5sqj8vc13h.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936) Bug#35996Martin Hansson21 Aug