From f99586668a79dd571a7dee7bce916d6b894d072a Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Fri, 11 Apr 2025 08:28:42 +0200 Subject: [PATCH] MDEV-36380 User has unauthorized access to a sequence through a view with security invoker check sequence privileges in Item_func_nextval::fix_fields(), just like column privileges are checked in Item_field::fix_fields() remove sequence specific hacks that kinda made sequence privilege checks works, but not in all cases. And they were too lax, didn't requre SELECT privilege for NEXTVAL. Also INSERT privilege looks wrong here, UPDATE would've been more appropriate, but won't change that for compatibility reasons. also fixes MDEV-36413 User without any privileges to a sequence can read from it and modify it via column default --- mysql-test/main/view_grant.result | 46 +++++++++++++++++ mysql-test/main/view_grant.test | 47 +++++++++++++++++ mysql-test/suite/sql_sequence/grant.result | 47 ++++++++++++++++- mysql-test/suite/sql_sequence/grant.test | 50 ++++++++++++++++++- mysql-test/suite/sql_sequence/gtid.result | 2 +- .../suite/sql_sequence/replication.result | 2 +- mysql-test/suite/sql_sequence/view.test | 1 - sql/item_func.cc | 10 ++++ sql/item_func.h | 7 +++ sql/sql_acl.cc | 14 ++---- sql/sql_parse.cc | 11 +--- 11 files changed, 211 insertions(+), 26 deletions(-) diff --git a/mysql-test/main/view_grant.result b/mysql-test/main/view_grant.result index f1bebf98ecd..57c0a0fcc55 100644 --- a/mysql-test/main/view_grant.result +++ b/mysql-test/main/view_grant.result @@ -1985,4 +1985,50 @@ connection default; DROP VIEW v1; DROP USER foo; DROP USER FOO; +# +# MDEV-36380: User has unauthorized access to a sequence through +# a view with security invoker +# +create database db; +use db; +create sequence s; +create sql security invoker view vin as select nextval(s); +create sql security definer view vdn as select nextval(s); +create sql security invoker view vil as select lastval(s); +create sql security definer view vdl as select lastval(s); +create sql security invoker view vis as select setval(s,20); +create sql security definer view vds as select setval(s,30); +create user u@localhost; +grant select on db.vin to u@localhost; +grant select on db.vdn to u@localhost; +grant select on db.vil to u@localhost; +grant select on db.vdl to u@localhost; +grant select on db.vis to u@localhost; +grant select on db.vds to u@localhost; +connect con1,localhost,u,,db; +select nextval(s); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vin; +ERROR HY000: View 'db.vin' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vdn; +nextval(s) +1 +select lastval(s); +ERROR 42000: SELECT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vil; +ERROR HY000: View 'db.vil' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vdl; +lastval(s) +1 +select setval(s,10); +ERROR 42000: INSERT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vis; +ERROR HY000: View 'db.vis' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vds; +setval(s,30) +30 +disconnect con1; +connection default; +drop database db; +drop user u@localhost; # End of 10.5 tests diff --git a/mysql-test/main/view_grant.test b/mysql-test/main/view_grant.test index fc0521933cc..d61362b6cc3 100644 --- a/mysql-test/main/view_grant.test +++ b/mysql-test/main/view_grant.test @@ -2239,4 +2239,51 @@ DROP VIEW v1; DROP USER foo; DROP USER FOO; +--echo # +--echo # MDEV-36380: User has unauthorized access to a sequence through +--echo # a view with security invoker +--echo # +create database db; +use db; +create sequence s; +create sql security invoker view vin as select nextval(s); +create sql security definer view vdn as select nextval(s); +create sql security invoker view vil as select lastval(s); +create sql security definer view vdl as select lastval(s); +create sql security invoker view vis as select setval(s,20); +create sql security definer view vds as select setval(s,30); +create user u@localhost; +grant select on db.vin to u@localhost; +grant select on db.vdn to u@localhost; +grant select on db.vil to u@localhost; +grant select on db.vdl to u@localhost; +grant select on db.vis to u@localhost; +grant select on db.vds to u@localhost; + +--connect (con1,localhost,u,,db) +--error ER_TABLEACCESS_DENIED_ERROR +select nextval(s); +--error ER_VIEW_INVALID +select * from vin; +--disable_ps2_protocol +select * from vdn; +--enable_ps2_protocol + +--error ER_TABLEACCESS_DENIED_ERROR +select lastval(s); +--error ER_VIEW_INVALID +select * from vil; +select * from vdl; + +--error ER_TABLEACCESS_DENIED_ERROR +select setval(s,10); +--error ER_VIEW_INVALID +select * from vis; +select * from vds; + +--disconnect con1 +--connection default +drop database db; +drop user u@localhost; + --echo # End of 10.5 tests diff --git a/mysql-test/suite/sql_sequence/grant.result b/mysql-test/suite/sql_sequence/grant.result index 0a69d69fc74..fc3421efcb6 100644 --- a/mysql-test/suite/sql_sequence/grant.result +++ b/mysql-test/suite/sql_sequence/grant.result @@ -47,14 +47,57 @@ next_not_cached_value minimum_value maximum_value start_value increment cache_si 11 1 9223372036854775806 1 1 1000 0 0 connection only_alter; select next value for s1; -ERROR 42000: INSERT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` alter sequence s1 restart= 11; select * from s1; ERROR 42000: SELECT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` connection default; -drop database mysqltest_1; drop user 'normal'@'%'; drop user 'read_only'@'%'; drop user 'read_write'@'%'; drop user 'alter'@'%'; drop user 'only_alter'@'%'; +drop sequence s1; +# +# MDEV-36413 User without any privileges to a sequence can read from +# it and modify it via column default +# +create sequence s1; +create sequence s2; +select * from s2; +next_not_cached_value minimum_value maximum_value start_value increment cache_size cycle_option cycle_count +1 1 9223372036854775806 1 1 1000 0 0 +create table t2 (a int not null default(nextval(s1))); +insert into t2 values(); +create user u; +grant create, insert, select, drop on mysqltest_1.t1 to u; +grant insert, select on mysqltest_1.s1 to u; +grant select on mysqltest_1.t2 to u; +connect con1,localhost,u,,mysqltest_1; +select nextval(s2); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +show create sequence s2; +ERROR 42000: SHOW command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +create table t1 (a int not null default(nextval(s1))); +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from t2; +insert into t1 values(); +select * from t1; +a +1 +2 +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from (select t2.a from t2,t2 as t3 where t2.a=t3.a) as t4; +drop table t1; +create table t1 (a int not null default(nextval(s2))); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +create table t1 (a int not null default(nextval(s1)), +b int not null default(nextval(s2))); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +disconnect con1; +connection default; +drop user u; +drop database mysqltest_1; +# +# End of 10.11 tests +# diff --git a/mysql-test/suite/sql_sequence/grant.test b/mysql-test/suite/sql_sequence/grant.test index fb8a9f813a6..c205bd34223 100644 --- a/mysql-test/suite/sql_sequence/grant.test +++ b/mysql-test/suite/sql_sequence/grant.test @@ -60,10 +60,58 @@ select * from s1; # connection default; -drop database mysqltest_1; drop user 'normal'@'%'; drop user 'read_only'@'%'; drop user 'read_write'@'%'; drop user 'alter'@'%'; drop user 'only_alter'@'%'; +drop sequence s1; +--echo # +--echo # MDEV-36413 User without any privileges to a sequence can read from +--echo # it and modify it via column default +--echo # + +create sequence s1; +create sequence s2; +select * from s2; +create table t2 (a int not null default(nextval(s1))); +insert into t2 values(); + +create user u; +grant create, insert, select, drop on mysqltest_1.t1 to u; +grant insert, select on mysqltest_1.s1 to u; +grant select on mysqltest_1.t2 to u; + +--connect(con1,localhost,u,,mysqltest_1) +--error ER_TABLEACCESS_DENIED_ERROR +select nextval(s2); +--error ER_TABLEACCESS_DENIED_ERROR +show create sequence s2; + +create table t1 (a int not null default(nextval(s1))); +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from t2; +insert into t1 values(); +select * from t1; +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from (select t2.a from t2,t2 as t3 where t2.a=t3.a) as t4; +drop table t1; +--error ER_TABLEACCESS_DENIED_ERROR +create table t1 (a int not null default(nextval(s2))); +--error ER_TABLEACCESS_DENIED_ERROR +create table t1 (a int not null default(nextval(s1)), + b int not null default(nextval(s2))); +--disconnect con1 +--connection default +drop user u; + +# +# Cleanup +# + +drop database mysqltest_1; + +--echo # +--echo # End of 10.11 tests +--echo # diff --git a/mysql-test/suite/sql_sequence/gtid.result b/mysql-test/suite/sql_sequence/gtid.result index 5c0003d4ea3..f59dc5595ee 100644 --- a/mysql-test/suite/sql_sequence/gtid.result +++ b/mysql-test/suite/sql_sequence/gtid.result @@ -174,7 +174,7 @@ create sequence s_db.s2; drop sequence s_db.s2; connection m_normal_2; select next value for s_db.s1; -ERROR 42000: INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` create sequence s_db.s2; ERROR 42000: CREATE command denied to user 'normal_2'@'localhost' for table `s_db`.`s2` connection m_normal_1; diff --git a/mysql-test/suite/sql_sequence/replication.result b/mysql-test/suite/sql_sequence/replication.result index 762c332dbd6..0e4ca1c939e 100644 --- a/mysql-test/suite/sql_sequence/replication.result +++ b/mysql-test/suite/sql_sequence/replication.result @@ -285,7 +285,7 @@ create sequence s_db.s2; drop sequence s_db.s2; connection m_normal_2; select NEXT VALUE for s_db.s1; -ERROR 42000: INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` create sequence s_db.s2; ERROR 42000: CREATE command denied to user 'normal_2'@'localhost' for table `s_db`.`s2` connection m_normal_1; diff --git a/mysql-test/suite/sql_sequence/view.test b/mysql-test/suite/sql_sequence/view.test index affac002878..7e2cd712325 100644 --- a/mysql-test/suite/sql_sequence/view.test +++ b/mysql-test/suite/sql_sequence/view.test @@ -1,5 +1,4 @@ --source include/have_sequence.inc ---source include/have_innodb.inc # # Test sequences with views diff --git a/sql/item_func.cc b/sql/item_func.cc index 6a7b12ae4a0..3ca3d5af3a3 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7092,6 +7092,16 @@ longlong Item_func_cursor_rowcount::val_int() /***************************************************************************** SEQUENCE functions *****************************************************************************/ +bool Item_func_nextval::check_access_and_fix_fields(THD *thd, Item **ref, + privilege_t want_access) +{ + table_list->sequence= false; + bool error= check_single_table_access(thd, want_access, table_list, false); + table_list->sequence= true; + if (error && table_list->belong_to_view) + table_list->hide_view_error(thd); + return error || Item_longlong_func::fix_fields(thd, ref); +} longlong Item_func_nextval::val_int() { diff --git a/sql/item_func.h b/sql/item_func.h index f4f0558a77b..68b3669c3b4 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -3774,11 +3774,14 @@ class Item_func_nextval :public Item_longlong_func protected: TABLE_LIST *table_list; TABLE *table; + bool check_access_and_fix_fields(THD *, Item **ref, privilege_t); public: Item_func_nextval(THD *thd, TABLE_LIST *table_list_arg): Item_longlong_func(thd), table_list(table_list_arg) {} longlong val_int() override; const char *func_name() const override { return "nextval"; } + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, INSERT_ACL | SELECT_ACL); } bool fix_length_and_dec() override { unsigned_flag= 0; @@ -3820,6 +3823,8 @@ class Item_func_lastval :public Item_func_nextval public: Item_func_lastval(THD *thd, TABLE_LIST *table_list_arg): Item_func_nextval(thd, table_list_arg) {} + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, SELECT_ACL); } longlong val_int() override; const char *func_name() const override { return "lastval"; } Item *do_get_copy(THD *thd) const override @@ -3840,6 +3845,8 @@ public: : Item_func_nextval(thd, table_list_arg), nextval(nextval_arg), round(round_arg), is_used(is_used_arg) {} + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, INSERT_ACL); } longlong val_int() override; const char *func_name() const override { return "setval"; } void print(String *str, enum_query_type query_type) override; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 385d3da18e2..939061fdf8e 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -8304,19 +8304,13 @@ bool check_grant(THD *thd, privilege_t want_access, TABLE_LIST *tables, /* If sequence is used as part of NEXT VALUE, PREVIOUS VALUE or SELECT, - we need to modify the requested access rights depending on how the - sequence is used. + the privilege will be checked in ::fix_fields(). + Direct SELECT of a sequence table doesn't set t_ref->sequence, so + privileges will be checked normally, as for any table. */ if (t_ref->sequence && !(want_access & ~(SELECT_ACL | INSERT_ACL | UPDATE_ACL | DELETE_ACL))) - { - /* - We want to have either SELECT or INSERT rights to sequences depending - on how they are accessed - */ - orig_want_access= ((t_ref->lock_type == TL_WRITE_ALLOW_WRITE) ? - INSERT_ACL : SELECT_ACL); - } + continue; const ACL_internal_table_access *access= get_cached_table_access(&t_ref->grant.m_internal, diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 705dc35cdb7..b017cba3c36 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7332,18 +7332,9 @@ check_table_access(THD *thd, privilege_t requirements, TABLE_LIST *tables, DBUG_PRINT("info", ("derived: %d view: %d", table_ref->derived != 0, table_ref->view != 0)); - if (table_ref->is_anonymous_derived_table()) + if (table_ref->is_anonymous_derived_table() || table_ref->sequence) continue; - if (table_ref->sequence) - { - /* We want to have either SELECT or INSERT rights to sequences depending - on how they are accessed - */ - want_access= ((table_ref->lock_type == TL_WRITE_ALLOW_WRITE) ? - INSERT_ACL : SELECT_ACL); - } - if (check_access(thd, want_access, table_ref->get_db_name().str, &table_ref->grant.privilege, &table_ref->grant.m_internal,