BUG#18681: View privileges are broken

The check for view security was lacking several points :
1. Check with the right set of permissions : for each table ref that
participates in a view there were the right credentials to use in it's
security_ctx member, but these weren't used for checking the credentials.
This makes hard enforcing the SQL SECURITY DEFINER|INVOKER property
consistently.
2. Because of the above the security checking for views was just ruled out
in explicit ways in several places.
3. The security was checked only for the columns of the tables that are
brought into the query from a view. So if there is no column reference
outside of the view definition it was not detecting the lack of access to
the tables in the view in SQL SECURITY INVOKER mode.

The fix below tries to fix the above 3 points.


mysql-test/r/grant.result:
  removed nondeterminism (unspecified order) in some test output
mysql-test/r/view_grant.result:
  Somewhat extended test case for the bug and similar queries.
mysql-test/t/grant.test:
  removed nondeterminism (unspecified order) in some test output
mysql-test/t/view_grant.test:
  Somewhat extended test case for the bug and similar queries.
sql/mysql_priv.h:
  A wrapper for setup_tables that also checks access to the tables
sql/sql_acl.cc:
  removed artificial security check stop and used the table ref's credentials.
sql/sql_base.cc:
  a wrapper for setup_tables to check access to the tables
sql/sql_delete.cc:
  wrapper called.
sql/sql_insert.cc:
  wrapper called
sql/sql_load.cc:
  wrapper called
sql/sql_parse.cc:
  wrapper called and artificial check stop removed
sql/sql_select.cc:
  wrapper called
sql/sql_update.cc:
  wrapper called
sql/table.cc:
  Mask table access to the view error as well.

10 files changed, 136 insertions, 40 deletions

diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 815876688fb..0e90b609f8c 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -946,6 +946,13 @@ bool insert_fields(THD *thd, Name_resolution_context *context,
 bool setup_tables(THD *thd, Name_resolution_context *context,
                   List<TABLE_LIST> *from_clause, TABLE_LIST *tables,
                   Item **conds, TABLE_LIST **leaves, bool select_insert);
+bool setup_tables_and_check_access (THD *thd, 
+                                    Name_resolution_context *context,
+                                    List<TABLE_LIST> *from_clause, 
+                                    TABLE_LIST *tables, Item **conds, 
+                                    TABLE_LIST **leaves, 
+                                    bool select_insert,
+                                    ulong want_access);
 int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
 	       List<Item> *sum_func_list, uint wild_num);
 bool setup_fields(THD *thd, Item** ref_pointer_array,
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 5b40f93f002..e84bb3c1dd7 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -3562,6 +3562,7 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
   TABLE_LIST *table, *first_not_own_table= thd->lex->first_not_own_table();
   Security_context *sctx= thd->security_ctx;
   uint i;
+  ulong orig_want_access= want_access;
   DBUG_ENTER("check_grant");
   DBUG_ASSERT(number > 0);
 
@@ -3583,18 +3584,22 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables,
     table->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
   }
 
-  want_access&= ~sctx->master_access;
-  if (!want_access)
-    DBUG_RETURN(0);                             // ok
-
   rw_rdlock(&LOCK_grant);
   for (table= tables;
        table && number-- && table != first_not_own_table;
        table= table->next_global)
   {
     GRANT_TABLE *grant_table;
+    sctx = test(table->security_ctx) ?
+      table->security_ctx : thd->security_ctx;
+
+    want_access= orig_want_access;
+    want_access&= ~sctx->master_access;
+    if (!want_access)
+      continue;                                 // ok
+
     if (!(~table->grant.privilege & want_access) || 
-        table->derived || table->schema_table || table->belong_to_view)
+        table->derived || table->schema_table)
     {
       /*
         It is subquery in the FROM clause. VIEW set table->derived after
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 1f3b9e14631..c667b902f51 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -4498,6 +4498,58 @@ bool setup_tables(THD *thd, Name_resolution_context *context,
 
 
 /*
+  prepare tables and check access for the view tables
+
+  SYNOPSIS
+    setup_tables_and_check_view_access()
+    thd		  Thread handler
+    context       name resolution contest to setup table list there
+    from_clause   Top-level list of table references in the FROM clause
+    tables	  Table list (select_lex->table_list)
+    conds	  Condition of current SELECT (can be changed by VIEW)
+    leaves        List of join table leaves list (select_lex->leaf_tables)
+    refresh       It is onle refresh for subquery
+    select_insert It is SELECT ... INSERT command
+    want_access   what access is needed
+
+  NOTE
+    a wrapper for check_tables that will also check the resulting
+    table leaves list for access to all the tables that belong to a view
+
+  RETURN
+    FALSE ok;  In this case *map will include the chosen index
+    TRUE  error
+*/
+bool setup_tables_and_check_access(THD *thd, 
+                                   Name_resolution_context *context,
+                                   List<TABLE_LIST> *from_clause,
+                                   TABLE_LIST *tables,
+                                   Item **conds, TABLE_LIST **leaves,
+                                   bool select_insert,
+                                   ulong want_access)
+{
+  TABLE_LIST *leaves_tmp = NULL;
+
+  if (setup_tables (thd, context, from_clause, tables, conds, 
+                    &leaves_tmp, select_insert))
+    return TRUE;
+
+  if (leaves)
+    *leaves = leaves_tmp;
+
+  for (; leaves_tmp; leaves_tmp= leaves_tmp->next_leaf)
+    if (leaves_tmp->belong_to_view && 
+        check_one_table_access(thd, want_access,  leaves_tmp))
+    {
+      tables->hide_view_error(thd);
+      return TRUE;
+    }
+
+  return FALSE;
+}
+
+
+/*
    Create a key_map from a list of index names
 
    SYNOPSIS
diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
index 37c4f9a3256..af20b770c56 100644
--- a/sql/sql_delete.cc
+++ b/sql/sql_delete.cc
@@ -334,10 +334,11 @@ bool mysql_prepare_delete(THD *thd, TABLE_LIST *table_list, Item **conds)
   DBUG_ENTER("mysql_prepare_delete");
 
   thd->lex->allow_sum_func= 0;
-  if (setup_tables(thd, &thd->lex->select_lex.context,
-                   &thd->lex->select_lex.top_join_list,
-                   table_list, conds, &select_lex->leaf_tables,
-                   FALSE) ||
+  if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context,
+                                    &thd->lex->select_lex.top_join_list,
+                                    table_list, conds, 
+                                    &select_lex->leaf_tables, FALSE, 
+                                    DELETE_ACL) ||
       setup_conds(thd, table_list, select_lex->leaf_tables, conds) ||
       setup_ftfuncs(select_lex))
     DBUG_RETURN(TRUE);
@@ -396,10 +397,11 @@ bool mysql_multi_delete_prepare(THD *thd)
 
     lex->query_tables also point on local list of DELETE SELECT_LEX
   */
-  if (setup_tables(thd, &thd->lex->select_lex.context,
-                   &thd->lex->select_lex.top_join_list,
-                   lex->query_tables, &lex->select_lex.where,
-                   &lex->select_lex.leaf_tables, FALSE))
+  if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context,
+                                    &thd->lex->select_lex.top_join_list,
+                                    lex->query_tables, &lex->select_lex.where,
+                                    &lex->select_lex.leaf_tables, FALSE, 
+                                    DELETE_ACL))
     DBUG_RETURN(TRUE);
 
 
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index c054499ecd8..efe8a1bca00 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -743,10 +743,11 @@ static bool mysql_prepare_insert_check_table(THD *thd, TABLE_LIST *table_list,
   bool insert_into_view= (table_list->view != 0);
   DBUG_ENTER("mysql_prepare_insert_check_table");
 
-  if (setup_tables(thd, &thd->lex->select_lex.context,
-                   &thd->lex->select_lex.top_join_list,
-                   table_list, where, &thd->lex->select_lex.leaf_tables,
-		   select_insert))
+  if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context,
+                                    &thd->lex->select_lex.top_join_list,
+                                    table_list, where, 
+                                    &thd->lex->select_lex.leaf_tables,
+                                    select_insert, INSERT_ACL))
     DBUG_RETURN(TRUE);
 
   if (insert_into_view && !fields.elements)
diff --git a/sql/sql_load.cc b/sql/sql_load.cc
index 0a667c887ef..eaee5edf9f1 100644
--- a/sql/sql_load.cc
+++ b/sql/sql_load.cc
@@ -153,10 +153,11 @@ bool mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list,
   ha_enable_transaction(thd, FALSE); 
   if (open_and_lock_tables(thd, table_list))
     DBUG_RETURN(TRUE);
-  if (setup_tables(thd, &thd->lex->select_lex.context,
-                   &thd->lex->select_lex.top_join_list,
-                   table_list, &unused_conds,
-		   &thd->lex->select_lex.leaf_tables, FALSE))
+  if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context,
+                                    &thd->lex->select_lex.top_join_list,
+                                    table_list, &unused_conds,
+                                    &thd->lex->select_lex.leaf_tables, FALSE,
+                                    INSERT_ACL | UPDATE_ACL))
      DBUG_RETURN(-1);
   if (!table_list->table ||               // do not suport join view
       !table_list->updatable ||           // and derived tables
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 7d62e5bb405..37e45e999b3 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -4994,23 +4994,35 @@ error:
 
 bool check_one_table_access(THD *thd, ulong privilege, TABLE_LIST *all_tables)
 {
+  Security_context * backup_ctx= thd->security_ctx;
+
+  /* we need to switch to the saved context (if any) */
+  if (all_tables->security_ctx)
+    thd->security_ctx= all_tables->security_ctx;
+
   if (check_access(thd, privilege, all_tables->db,
 		   &all_tables->grant.privilege, 0, 0,
                    test(all_tables->schema_table)))
-    return 1;
+    goto deny;
 
   /* Show only 1 table for check_grant */
   if (grant_option && check_grant(thd, privilege, all_tables, 0, 1, 0))
-    return 1;
+    goto deny;
+
+  thd->security_ctx= backup_ctx;
 
   /* Check rights on tables of subselects and implictly opened tables */
   TABLE_LIST *subselects_tables;
   if ((subselects_tables= all_tables->next_global))
   {
     if ((check_table_access(thd, SELECT_ACL, subselects_tables, 0)))
-      return 1;
+      goto deny;
   }
   return 0;
+
+deny:
+  thd->security_ctx= backup_ctx;
+  return 1;
 }
 
 
@@ -5191,6 +5203,7 @@ check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
   ulong found_access=0;
   TABLE_LIST *org_tables= tables;
   TABLE_LIST *first_not_own_table= thd->lex->first_not_own_table();
+  Security_context *sctx= thd->security_ctx, *backup_ctx= thd->security_ctx;
   /*
     The check that first_not_own_table is not reached is for the case when
     the given table list refers to the list for prelocking (contains tables
@@ -5198,12 +5211,17 @@ check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
   */
   for (; tables != first_not_own_table; tables= tables->next_global)
   {
+    if (tables->security_ctx)
+      sctx= tables->security_ctx;
+    else
+      sctx= backup_ctx;
+
     if (tables->schema_table && 
         (want_access & ~(SELECT_ACL | EXTRA_ACL | FILE_ACL)))
     {
       if (!no_errors)
         my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
-                 thd->security_ctx->priv_user, thd->security_ctx->priv_host,
+                 sctx->priv_user, sctx->priv_host,
                  information_schema_name.str);
       return TRUE;
     }
@@ -5212,12 +5230,13 @@ check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
        Remove SHOW_VIEW_ACL, because it will be checked during making view
      */
     tables->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
-    if (tables->derived || tables->schema_table || tables->belong_to_view ||
+    if (tables->derived || tables->schema_table ||
         (tables->table && (int)tables->table->s->tmp_table) ||
         my_tz_check_n_skip_implicit_tables(&tables,
                                            thd->lex->time_zone_tables_used))
       continue;
-    if ((thd->security_ctx->master_access & want_access) ==
+    thd->security_ctx= sctx;
+    if ((sctx->master_access & want_access) ==
         (want_access & ~EXTRA_ACL) &&
 	thd->db)
       tables->grant.privilege= want_access;
@@ -5229,19 +5248,23 @@ check_table_access(THD *thd, ulong want_access,TABLE_LIST *tables,
       {
 	if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
 			 0, no_errors, test(tables->schema_table)))
-	  return TRUE;				// Access denied
+	  goto deny;                            // Access denied
 	found_access=tables->grant.privilege;
 	found=1;
       }
     }
     else if (check_access(thd,want_access,tables->db,&tables->grant.privilege,
 			  0, no_errors, test(tables->schema_table)))
-      return TRUE;
+      goto deny;
   }
+  thd->security_ctx= backup_ctx;
   if (grant_option)
     return check_grant(thd,want_access & ~EXTRA_ACL,org_tables,
 		       test(want_access & EXTRA_ACL), UINT_MAX, no_errors);
   return FALSE;
+deny:
+  thd->security_ctx= backup_ctx;
+  return TRUE;
 }
 
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index d36ceedc7ae..9705cd2643f 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -337,9 +337,10 @@ JOIN::prepare(Item ***rref_pointer_array,
   /* Check that all tables, fields, conds and order are ok */
 
   if ((!(select_options & OPTION_SETUP_TABLES_DONE) &&
-       setup_tables(thd, &select_lex->context, join_list,
-                    tables_list, &conds, &select_lex->leaf_tables,
-                    FALSE)) ||
+       setup_tables_and_check_access(thd, &select_lex->context, join_list,
+                                     tables_list, &conds, 
+                                     &select_lex->leaf_tables, FALSE, 
+                                     SELECT_ACL)) ||
       setup_wild(thd, tables_list, fields_list, &all_fields, wild_num) ||
       select_lex->setup_ref_array(thd, og_num) ||
       setup_fields(thd, (*rref_pointer_array), fields_list, 1,
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index dfe23c9a503..b4ae779f9e2 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -613,9 +613,11 @@ bool mysql_prepare_update(THD *thd, TABLE_LIST *table_list,
   tables.alias= table_list->alias;
   thd->lex->allow_sum_func= 0;
 
-  if (setup_tables(thd, &select_lex->context, &select_lex->top_join_list,
-                   table_list, conds, &select_lex->leaf_tables,
-                   FALSE) ||
+  if (setup_tables_and_check_access(thd, &select_lex->context, 
+                                    &select_lex->top_join_list,
+                                    table_list, conds, 
+                                    &select_lex->leaf_tables,
+                                    FALSE, UPDATE_ACL) ||
       setup_conds(thd, table_list, select_lex->leaf_tables, conds) ||
       select_lex->setup_ref_array(thd, order_num) ||
       setup_order(thd, select_lex->ref_pointer_array,
@@ -706,10 +708,11 @@ reopen_tables:
     call in setup_tables()).
   */
 
-  if (setup_tables(thd, &lex->select_lex.context,
-                   &lex->select_lex.top_join_list,
-                   table_list, &lex->select_lex.where,
-                   &lex->select_lex.leaf_tables, FALSE))
+  if (setup_tables_and_check_access(thd, &lex->select_lex.context,
+                                    &lex->select_lex.top_join_list,
+                                    table_list, &lex->select_lex.where,
+                                    &lex->select_lex.leaf_tables, FALSE,
+                                    UPDATE_ACL))
     DBUG_RETURN(TRUE);
 
   if (setup_fields_with_no_wrap(thd, 0, *fields, 1, 0, 0))
diff --git a/sql/table.cc b/sql/table.cc
index 8e23bea2540..b3ed5ab91ee 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -2068,7 +2068,8 @@ void st_table_list::hide_view_error(THD *thd)
   if (thd->net.last_errno == ER_BAD_FIELD_ERROR ||
       thd->net.last_errno == ER_SP_DOES_NOT_EXIST ||
       thd->net.last_errno == ER_PROCACCESS_DENIED_ERROR ||
-      thd->net.last_errno == ER_COLUMNACCESS_DENIED_ERROR)
+      thd->net.last_errno == ER_COLUMNACCESS_DENIED_ERROR ||
+      thd->net.last_errno == ER_TABLEACCESS_DENIED_ERROR)
   {
     TABLE_LIST *top= top_table();
     thd->clear_error();