Details
-
Bug
-
Status: Closed (View Workflow)
-
Blocker
-
Resolution: Fixed
-
10.5, 10.6, 10.11, 11.4, 11.8
-
None
Description
Courtesy of wlad
create database db; |
create user u; |
grant create, insert, select on db.t to u; |
create sequence db.s; |
select * from db.s; |
|
|
--connect(con1,localhost,u,,db)
|
--error ER_TABLEACCESS_DENIED_ERROR
|
select nextval(s); |
--error ER_TABLEACCESS_DENIED_ERROR
|
show create sequence s; |
|
|
create table t (a int not null default(nextval(s))); |
insert into t values (),(); |
select * from t; |
select default(a) from t; |
--disconnect con1
|
|
|
--connection default
|
select * from db.s; |
drop database db; |
drop user u; |
|
10.5 85ecb80fa3192035b3beff9578dc254a857175dc |
connect con1,localhost,u,,db; |
select nextval(s); |
ERROR 42000: INSERT command denied to user 'u'@'localhost' for table `db`.`s` |
show create sequence s; |
ERROR 42000: SHOW command denied to user 'u'@'localhost' for table `db`.`s` |
create table t (a int not null default(nextval(s))); |
insert into t values (),(); |
select * from t; |
a
|
1
|
2
|
select default(a) from t; |
default(a) |
3
|
4
|
disconnect con1;
|
So, the user u does not have any access to the sequence db.s, cannot read from it or see its structure.
However, the user can refer to it in CREATE TABLE, and can read/insert a default value in such a table, thus both reading and modifying the sequence.
It has been discussed that the privileges should be checked at some point. It can be done
1) upon table creation/altering, or
2) upon insert/select.
Either way is likely to break something for existing users. Probably the first way, checking privileges upon creation, will break less, at least because this is a less frequent operation than INSERT/SELECT, but it's hard to predict for sure.
Another argument for the first approach is that the existence of the sequence is already checked upon CREATE, one can't create a table referring a sequence in DEFAULT if the sequence doesn't exist or if the referred object is not a sequence, which also means that in the variation of the test case above the unprivileged user gets information about db.s which they shouldn't normally have:
create user u; |
grant create, insert, select on db.t to u; |
create table db.s (x int); |
connect con1,localhost,u,,db; |
create table t (a int not null default(nextval(s))); |
ERROR 42S02: 'db.s' is not a SEQUENCE |
create table t (a int not null default(nextval(s2))); |
ERROR 42S02: Table 'db.s2' doesn't exist |
Attachments
Issue Links
- blocks
-
-
- In Review
-
- relates to
-
-
- Confirmed
-
-
-
- Closed
-
Activity
Another argument for the "upon creation" approach is that a user doesn't need to have a privilege on a column to insert a default value into the column, so it would be strange if it required a privilege on the underlying function which generates this value.
Pushed to bb-10.11-monty. This is not critical enough to be pushed to 10.6.
Found one issue with CREATE TABLE and prepared statements, where the prepared statement changes the lex table lists in an incompatible way compared to normal execution. I have created a patch for this, but need to have the patch reviewed by Sanja as it is hard to know that it is 100% correct.
The problem can be seen by running mtr --ps sql_sequence.grant in the bb-10.11-monty2 tree. This fails because the code expects the wrong grants for the used tables.
The bug is that TABLE_LIST *LEX::unlink_first_table() and LEX::link_first_table_back() does not restore back the original table lists because of the call to first_lists_tables_same() in unlink_first_table() which irrevocable changes the table lists.
I do not like that we have to call unlink - link_back in many places of the code, just because of some statements has an indirect SELECT attached (CREATE ... SELECT, INSERT ... SELECT). It would be better that lex would keep the 'main tables' in a separate list so that we could use the SELECT part directly without ever have to modify the lists back and forth.
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
|
index f3d51db028d..b9c81e14ef7 100644
|
--- a/sql/sql_lex.cc
|
+++ b/sql/sql_lex.cc
|
@@ -1226,6 +1226,7 @@ void LEX::start(THD *thd_arg)
|
with_clauses_list_last_next= &with_clauses_list;
|
clone_spec_offset= 0;
|
create_view= NULL;
|
+ create_group= NULL;
|
field_list.empty();
|
value_list.empty();
|
update_list.empty();
|
@@ -4483,8 +4484,30 @@ void LEX::set_trg_event_type_for_tables()
|
|
|
/*
|
- Unlink the first table from the global table list and the first table from
|
- outer select (lex->select_lex) local list
|
+ It happens that when we parse CREATE TABLE at the first place goes
|
+ not only the table we are creating but also sequences used in this table
|
+ exprssions. This function called in the parser just after parsing the
|
+ table definition to mark of this group of tables
|
+*/
|
+
|
+void LEX::mark_create_group()
|
+{
|
+ create_group= query_tables;
|
+ if (query_tables)
|
+ {
|
+ while(create_group->next_global)
|
+ {
|
+ create_group= create_group->next_global;
|
+ // all sequences of CREATE are not linked locally
|
+ DBUG_ASSERT(create_group->next_local == NULL);
|
+ }
|
+ }
|
+}
|
+
|
+/*
|
+ Unlink the first table with its group (if present) from the global
|
+ table list and the first table from outer select (lex->select_lex)
|
+ local list
|
|
SYNOPSIS
|
unlink_first_table()
|
@@ -4506,13 +4529,14 @@ TABLE_LIST *LEX::unlink_first_table(bool *link_to_local)
|
if ((first= query_tables))
|
{
|
/*
|
- Exclude from global table list
|
+ Exclude the group from global table list (see mark_create_group)
|
*/
|
- if ((query_tables= query_tables->next_global))
|
+ TABLE_LIST *last= (create_group ? create_group : query_tables );
|
+ if ((query_tables= last->next_global))
|
query_tables->prev_global= &query_tables;
|
else
|
query_tables_last= &query_tables;
|
- first->next_global= 0;
|
+ last->next_global= 0;
|
|
/*
|
and from local list if it is not empty
|
@@ -4598,7 +4622,8 @@ void LEX::fix_first_select_number()
|
|
|
/*
|
- Link table back that was unlinked with unlink_first_table()
|
+ Link back ex-first table with its group that was unlinked with
|
+ unlink_first_table()
|
|
SYNOPSIS
|
link_first_table_back()
|
@@ -4613,10 +4638,12 @@ void LEX::link_first_table_back(TABLE_LIST *first,
|
{
|
if (first)
|
{
|
- if ((first->next_global= query_tables))
|
- query_tables->prev_global= &first->next_global;
|
+ // see mark_create_group for create_group explanation
|
+ TABLE_LIST *last= (create_group ? create_group : first);
|
+ if ((last->next_global= query_tables))
|
+ query_tables->prev_global= &last->next_global;
|
else
|
- query_tables_last= &first->next_global;
|
+ query_tables_last= &last->next_global;
|
query_tables= first;
|
|
if (link_to_local)
|
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
|
index 6312102e076..12affb42897 100644
|
--- a/sql/sql_lex.h
|
+++ b/sql/sql_lex.h
|
@@ -3230,6 +3230,7 @@ struct LEX: public Query_tables_list
|
my_ptrdiff_t clone_spec_offset;
|
|
Create_view_info *create_view;
|
+ TABLE_LIST *create_group;
|
|
/* Query Plan Footprint of a currently running select */
|
Explain_query *explain;
|
@@ -4934,6 +4935,7 @@ struct LEX: public Query_tables_list
|
builtin_select.options |= SELECT_DESCRIBE;
|
}
|
|
+ void mark_create_group();
|
};
|
|
|
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
|
index d5aeb57d546..bd6b5467114 100644
|
--- a/sql/sql_parse.cc
|
+++ b/sql/sql_parse.cc
|
@@ -10150,9 +10150,15 @@ bool create_table_precheck(THD *thd, TABLE_LIST *tables,
|
table && table != tables ;
|
table= table->next_global)
|
{
|
+ DBUG_ASSERT(table->sequence);
|
if (check_table_access(thd, INSERT_ACL | SELECT_ACL, table,
|
FALSE, 1, FALSE))
|
goto err;
|
+ // after last table in group starts other tables or it is the end
|
+ DBUG_ASSERT((table == lex->create_group &&
|
+ (table->next_global == NULL ||
|
+ table->next_global == tables)) ||
|
+ table != lex->create_group);
|
}
|
|
if (select_lex->item_list.elements)
|
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
|
index 2abd0a529af..6de89f05ffb 100644
|
--- a/sql/sql_yacc.yy
|
+++ b/sql/sql_yacc.yy
|
@@ -4472,7 +4472,10 @@ trg_event:
|
|
create_body:
|
create_field_list_parens
|
- { Lex->create_info.option_list= NULL; }
|
+ {
|
+ Lex->create_info.option_list= NULL;
|
+ Lex->mark_create_group();
|
+ }
|
opt_create_table_options opt_create_partitioning opt_create_select {}
|
| opt_create_table_options opt_create_partitioning opt_create_select {}
|
| create_like
|
Also, a table referencing another table is somewhat similar to foreign keys. And for them the privilege is checked on creation, not on every check.