Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-22374

VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE

Details

    • Bug
    • Status: Stalled (View Workflow)
    • Critical
    • Resolution: Unresolved
    • 5.5(EOL), 10.1(EOL), 10.2(EOL), 10.3(EOL), 10.4(EOL), 10.5, 10.6, 10.7(EOL)
    • 10.5, 10.6
    • Views
    • None

    Description

       
      create user test@localhost;
      grant select on test.* to test@localhost;
       
      create table t1 (a int);
      create definer=test@localhost sql security definer view v1 as select * from t1;
       
      --echo # check that ot works without view
      --eval select * INTO OUTFILE '$MYSQL_TMP_DIR/test_out_txt' from t1;
      --echo # check that ot works without file
      select * from v1;
       
      --echo # rights for file should be taken from current user not view
      --eval select * INTO OUTFILE '$MYSQL_TMP_DIR/test_out_txt' from (select count(*) from v1) as dv1;
      --echo # rights for file should be taken from current user not view
      --eval select * INTO OUTFILE '$MYSQL_TMP_DIR/test_out_txt' from (select * from v1) as dv1;
      --eval select * INTO OUTFILE '$MYSQL_TMP_DIR/test_out_txt' from v1;
       
      --remove_file $MYSQL_TMP_DIR/test_out_txt
      drop view v1;
      drop table t1;
      drop user test@localhost;
      

      Attachments

        Issue Links

          Activity

            sanja Oleksandr Byelkin created issue -
            sanja Oleksandr Byelkin made changes -
            Field Original Value New Value
            Assignee Oleksandr Byelkin [ sanja ]
            sanja Oleksandr Byelkin made changes -
            Component/s Views [ 10111 ]
            Fix Version/s 5.5 [ 15800 ]
            Fix Version/s 10.1 [ 16100 ]
            Fix Version/s 10.2 [ 14601 ]
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            sanja Oleksandr Byelkin made changes -
            sanja Oleksandr Byelkin made changes -
            Issue Type Task [ 3 ] Bug [ 1 ]
            sanja Oleksandr Byelkin made changes -
            Affects Version/s 5.5 [ 15800 ]
            Affects Version/s 10.1 [ 16100 ]
            Affects Version/s 10.2 [ 14601 ]
            Affects Version/s 10.3 [ 22126 ]
            Affects Version/s 10.4 [ 22408 ]
            Affects Version/s 10.5 [ 23123 ]
            sanja Oleksandr Byelkin made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            sanja Oleksandr Byelkin made changes -
            sanja Oleksandr Byelkin made changes -
            Status In Progress [ 3 ] Stalled [ 10000 ]
            sanja Oleksandr Byelkin made changes -
            Assignee Oleksandr Byelkin [ sanja ] Sergei Golubchik [ serg ]
            Status Stalled [ 10000 ] In Review [ 10002 ]

            commit d08860b28f3645bb59275941e56d63bef7ea3e05 (HEAD > bb-5.5MDEV-22374, origin/bb-5.5-MDEV-22374)
            Author: Oleksandr Byelkin <sanja@mariadb.com>
            Date: Tue Apr 28 09:16:33 2020 +0200

            MDEV-22374: VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE

            Check INTO OUTFILE clause always from invoker.

            sanja Oleksandr Byelkin added a comment - commit d08860b28f3645bb59275941e56d63bef7ea3e05 (HEAD > bb-5.5 MDEV-22374 , origin/bb-5.5- MDEV-22374 ) Author: Oleksandr Byelkin <sanja@mariadb.com> Date: Tue Apr 28 09:16:33 2020 +0200 MDEV-22374 : VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE Check INTO OUTFILE clause always from invoker.
            serg Sergei Golubchik made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            sanja Oleksandr Byelkin made changes -
            serg Sergei Golubchik made changes -
            serg Sergei Golubchik made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Oleksandr Byelkin [ sanja ]
            julien.fritsch Julien Fritsch made changes -
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 5.5 [ 15800 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.1 [ 16100 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 107792 ] MariaDB v4 [ 143637 ]
            alice Alice Sherepa made changes -
            Affects Version/s 10.6 [ 24028 ]
            Affects Version/s 10.7 [ 24805 ]
            alice Alice Sherepa made changes -
            Fix Version/s 10.6 [ 24028 ]
            Fix Version/s 10.7 [ 24805 ]
            serg Sergei Golubchik made changes -
            ralf.gebhardt Ralf Gebhardt made changes -
            Fix Version/s 10.2 [ 14601 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.7 [ 24805 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.3 [ 22126 ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.4 [ 22408 ]

            > +create table t1 (a int);
            > +create definer=test@localhost sql security definer view v1 as select * from t1;
            > +# check that ot works without view
             
            typo "it"
             
            > +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1;
            > +# check that ot works without file
             
            same
            why "//" ?
             
            > +select * from v1;
            > +a
            > +# rights for file should be taken from current user not view
            > +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1;
            > +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1;
            > +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1;
             
            same
             
            > +drop view v1;
             
            please add reverse tests too:
             
            +create sql security definer view v1 as select * from t1;
            +connect(con1, localhost, test);
            +error 1045;
            +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1;
            +select * from v1;
            +error 1045;
            +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1;
            +error 1045;
            +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1;
            +error 1045;
            +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1;
            +connection default;
             
            > +drop table t1;
            > +drop user test@localhost;
            > +# End of 5.5 tests
            > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
            > index ae5a6b4cd35..515990c879f 100644
            > --- a/sql/sql_parse.cc
            > +++ b/sql/sql_parse.cc
            > @@ -2206,15 +2206,15 @@ mysql_execute_command(THD *thd)
            >        lex->exchange != NULL implies SELECT .. INTO OUTFILE and this
            >        requires FILE_ACL access.
            >      */
            > -    ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL :
            > -      SELECT_ACL;
            > -
            > -    if (all_tables)
            > -      res= check_table_access(thd,
            > -                              privileges_requested,
            > -                              all_tables, FALSE, UINT_MAX, FALSE);
            > -    else
            > -      res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0);
            > +    if (lex->exchange)
            > +      res= check_access(thd, FILE_ACL, any_db, NULL, NULL, 0, 0);
            > +    if (!res)
            > +    {
            > +      if (all_tables)
            > +        res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, UINT_MAX, FALSE);
            > +      else
            > +        res= check_access(thd, SELECT_ACL, any_db, NULL, NULL, 0, 0);
            > +    }
             
            No, I think this special check for a special SELECT ... INTO OUTFILE
            case is wrong. Why does the server even checks global privileges via
            views? It should not do it, I think, not for FILE not for any other
            global or db level privilege. I'd try something like
             
            diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
            --- a/sql/sql_acl.cc
            +++ b/sql/sql_acl.cc
            @@ -4699,7 +4699,7 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST >
                   Save a copy of the privileges without the SHOW_VIEW_ACL attribute.
                   It will be checked during making view.
                 */
            -    tl->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
            +    tl->grant.orig_want_privilege= want_access & ~SHOW_VIEW_ACL & TABLE_ACLS;
               }
             
               mysql_rwlock_rdlock(&LOCK_grant);
            diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
            --- a/sql/sql_parse.cc
            +++ b/sql/sql_parse.cc
            @@ -5204,7 +5204,8 @@ check_table_access(THD *thd, ulong requirements,TABLE_LIS>
                    Register access for view underlying table.
                    Remove SHOW_VIEW_ACL, because it will be checked during making view
                  */
            -    table_ref->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL);
            +    table_ref->grant.orig_want_privilege=
            +      want_access & ~SHOW_VIEW_ACL & TABLE_ACLS;
             
                 if (table_ref->schema_table_reformed)
                 {
            

            sanja Oleksandr Byelkin added a comment - > +create table t1 (a int); > +create definer=test@localhost sql security definer view v1 as select * from t1; > +# check that ot works without view   typo "it"   > +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; > +# check that ot works without file   same why "//" ?   > +select * from v1; > +a > +# rights for file should be taken from current user not view > +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1;   same   > +drop view v1;   please add reverse tests too:   +create sql security definer view v1 as select * from t1; +connect(con1, localhost, test); +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; +select * from v1; +error 1045; +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1; +connection default;   > +drop table t1; > +drop user test@localhost; > +# End of 5.5 tests > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index ae5a6b4cd35..515990c879f 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -2206,15 +2206,15 @@ mysql_execute_command(THD *thd) > lex->exchange != NULL implies SELECT .. INTO OUTFILE and this > requires FILE_ACL access. > */ > - ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : > - SELECT_ACL; > - > - if (all_tables) > - res= check_table_access(thd, > - privileges_requested, > - all_tables, FALSE, UINT_MAX, FALSE); > - else > - res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0); > + if (lex->exchange) > + res= check_access(thd, FILE_ACL, any_db, NULL, NULL, 0, 0); > + if (!res) > + { > + if (all_tables) > + res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, UINT_MAX, FALSE); > + else > + res= check_access(thd, SELECT_ACL, any_db, NULL, NULL, 0, 0); > + }   No, I think this special check for a special SELECT ... INTO OUTFILE case is wrong. Why does the server even checks global privileges via views? It should not do it, I think, not for FILE not for any other global or db level privilege. I'd try something like   diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -4699,7 +4699,7 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST > Save a copy of the privileges without the SHOW_VIEW_ACL attribute. It will be checked during making view. */ - tl->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + tl->grant.orig_want_privilege= want_access & ~SHOW_VIEW_ACL & TABLE_ACLS; }   mysql_rwlock_rdlock(&LOCK_grant); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5204,7 +5204,8 @@ check_table_access(THD *thd, ulong requirements,TABLE_LIS> Register access for view underlying table. Remove SHOW_VIEW_ACL, because it will be checked during making view */ - table_ref->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + table_ref->grant.orig_want_privilege= + want_access & ~SHOW_VIEW_ACL & TABLE_ACLS;   if (table_ref->schema_table_reformed) {

            People

              sanja Oleksandr Byelkin
              sanja Oleksandr Byelkin
              Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.