List:Commits« Previous MessageNext Message »
From:Praveenkumar Hulakund Date:January 30 2012 7:01am
Subject:bzr push into mysql-trunk branch (praveenkumar.hulakund:3801 to 3802)
Bug#12602983
View as plain text  
 3802 Praveenkumar Hulakund	2012-01-30
      BUG#12602983 - USER WITHOUT PRIVILEGE ON ROUTINE CAN DISCOVER ITS EXISTENCE
                       (TAKE #2)
        
        Description:
        ------------------------------------------------
        User which doesn't have any privileges on the routine or on mysql.proc table 
        still is able to discover its existence. This is wrong as one should not know 
        anything about a database object unless one has privileges on it.
        
        Analysis:
        ------------------------------------------------
        The problem was, user without any privileges on routine was able to find
        out whether it existed or not. "select <func_name>" and "call <proc_name>" 
        were checking for the existence of the <func_name> or <proc_name>" before 
        checking whether user has enough privileges to execute function or not. 
        Error "<func_name> doesn't exists" or "<proc_name> doesn't exists" was 
        reported.
        
        For CREATE, ALTER, DROP we are already providing proper error
            DROP:
            ---------
            mysql> drop function mysqltest.f1;
            ERROR 1370 (42000): alter routine command denied to user ''@'localhost' for
                                routine 'mysqltest.f1'
            mysql> drop procedure  mysqltest.f1;
            ERROR 1370 (42000): alter routine command denied to user ''@'localhost' for
                                routine 'mysqltest.f1'
            
            CREATE:
            ----------
            mysql> create function mysqltest.f1() returns int return 0; 
            ERROR 1044 (42000): Access denied for user ''@'localhost' to database 
                                'mysqltest'
            mysql> create procedure mysqltest.p1() begin end; 
            ERROR 1044 (42000): Access denied for user ''@'localhost' to database 
                                'mysqltest'
            
            ALTER:
            ---------
            mysql> alter function mysqltest.f1 comment "TESTING";
            ERROR 1370 (42000): alter routine command denied to user ''@'localhost' for
                                routine 'mysqltest.f1'
            mysql> alter procedure  mysqltest.f1 comment "TESTING";
            ERROR 1370 (42000): alter routine command denied to user ''@'localhost' for 
                                routine 'mysqltest.f1'
        
        For "SELECT <function_name>" and "CALL <procedure_name>" we were 
        providing "doesn't exists" error.  Also when non existing function is used while
        creating the views we see same issue.
        
        Fix:
        ------------------------------------------------
        SELECT and CALL didn't have the logic to check execute privilege on routine for
        the user. This patch solves problem by checking the privileges to user before 
        checking the existence of the function.
     @ mysql-test/r/lowercase_fs_off.result
            Permission to execute procedure is verified before searching the procedure.
            Since, procedure name printed from the name specified in query now, new o/p
            has capital P in  db1.P1 (for the statement "call db1.P1")
     @ sql/item_func.cc
            For stored functions call in select and create view, checking the privilege to
            execute stored function before checking the existence of it by calling
            "check_routine_access".
     @ sql/sql_parse.cc
            Checking the privilege to execute stored procedure before checking the existence
            of it by calling "check_routine_access".

    modified:
      mysql-test/r/lowercase_fs_off.result
      mysql-test/r/sp-security.result
      mysql-test/t/sp-security.test
      sql/item_func.cc
      sql/sql_parse.cc
 3801 Chaithra Gopalareddy	2012-01-30
      Bug#11764651-57510:Comparing uninitialized values in CTYPE-BIN.c
      
      The first part of the problem reported in the bug, is  fixed in the patch for 
      Bug#11766212.This fix is for the second part of the problem reported.
      
      PROBLEM:
      
      The following query results in an invalid read when run with valgrind
      
      select left(geomfromtext("point(0 0)"),1) not in ( @@global.query_cache_type,1 not between -1 and "a", elt(1,'',1,1,1), geomfromtext("point(1 -1)") in ("bbbbbbbbb"),1);
      
      5.5.8-debug valgrind output:
      Invalid read of size 1
      at: my_strnncollsp_utf8 (ctype-utf8.c:2590)
      by: sortcmp (sql_string.cc:668)
      by: cmp_item_sort_string::cmp (item_cmpfunc.h:1019)
      by: Item_func_in::val_int (item_cmpfunc.cc:4130)
      by: Item::send (item.cc:5864)
      by: Protocol::send_result_set_row (protocol.cc:848)
      by: select_send::send_data (sql_class.cc:1789)
      by: JOIN::exec (sql_select.cc:1857)
      by: mysql_select (sql_select.cc:2568)
      by: handle_select (sql_select.cc:296)
      by: execute_sqlcom_select (sql_parse.cc:4464)
      by: mysql_execute_command (sql_parse.cc:2066)
      by: mysql_parse (sql_parse.cc:5500)
      by: dispatch_command (sql_parse.cc:1030)
      by: do_command (sql_parse.cc:770)
      by: do_handle_one_connection (sql_connect.cc:745)
      by: handle_one_connection (sql_connect.cc:684)
      by: start_thread (pthread_create.c:301)
      
      Problem Analysis:
      The above mentioned invalid read can be reproduced using a simplified query like this one:
      select left(geomfromtext("point(0 0)"),1) in (@@global.query_cache_type,1,"");
      
      Problem is present in cmp_item_sort_string::store_value, because it does not make 
      its own copy of the value evaluated. Instead tries to refer to a temporary value 
      that could be changed or deleted by other functions. Hence while reusing the 
      evaluated value in case of Item_func_in::val_int, we get a valgrind error saying 
      Invalid read. 
      
      With respect to the above query, this is what happens:
      
      a) To be compared with @@global.query_cache_type, function left() is evaluated in 
      "string" context. This evaluation modifies Item_func_left::tmp_value, and returns 
      a pointer to this tmp_value. 
      The "comparator object" (cmp_item) stores the returned pointer in 
      cmp_item::value_res and then the comparison is done. 
      
      b) To be compared with 1, left() is evaluated in "real number" context, which 
      modifies Item_func_left::tmp_value again. Comparison is done with the real value.
      
      c) To be compared with "", left() is NOT evaluated again because cmp_item already 
      has a pointer to the value of left() in string context (from (a)). However, this 
      value was a pointer to Item_func_left::tmp_value, and (b) has modified this String. So we would be  looking at out-of-date data. 
      Hence while reusing the evaluated value in case of Item_func_in::val_int, we get 
      a valgrind error saying Invalid read. 
      
      Solution:
      The fix it to make a copy of the value retured from val_str() in 
      cmp_item_sort_string::store_value().
     @ mysql-test/include/func_in.inc
        Added test case for Bug#11764651
     @ mysql-test/r/func_in_all.result
        Result file changes for the test case added
     @ mysql-test/r/func_in_icp.result
        Result file changes for the test case added
     @ mysql-test/r/func_in_icp_mrr.result
        Result file changes for the test case added
     @ mysql-test/r/func_in_mrr.result
        Result file changes for the test case added
     @ mysql-test/r/func_in_mrr_cost.result
        Result file changes for the test case added
     @ mysql-test/r/func_in_none.result
        Result file changes for the test case added
     @ sql/item_cmpfunc.h
        Changed cmp_item_sort_string::store_value() to make a copy of the evaluated result

    modified:
      mysql-test/include/func_in.inc
      mysql-test/r/func_in_all.result
      mysql-test/r/func_in_icp.result
      mysql-test/r/func_in_icp_mrr.result
      mysql-test/r/func_in_mrr.result
      mysql-test/r/func_in_mrr_cost.result
      mysql-test/r/func_in_none.result
      sql/item_cmpfunc.h
=== modified file 'mysql-test/r/lowercase_fs_off.result'
--- a/mysql-test/r/lowercase_fs_off.result	revid:chaithra.gopalareddy@stripped
+++ b/mysql-test/r/lowercase_fs_off.result	revid:praveenkumar.hulakund@stripped
@@ -44,7 +44,7 @@ f1(1)
 call p1();
 ERROR 42000: execute command denied to user 'USER_1'@'localhost' for routine 'db1.p1'
 call P1();
-ERROR 42000: execute command denied to user 'USER_1'@'localhost' for routine 'db1.p1'
+ERROR 42000: execute command denied to user 'USER_1'@'localhost' for routine 'db1.P1'
 select f1(1);
 ERROR 42000: execute command denied to user 'USER_1'@'localhost' for routine 'db1.f1'
 REVOKE ALL PRIVILEGES, GRANT OPTION FROM user_1@localhost;

=== modified file 'mysql-test/r/sp-security.result'
--- a/mysql-test/r/sp-security.result	revid:chaithra.gopalareddy@stripped
+++ b/mysql-test/r/sp-security.result	revid:praveenkumar.hulakund@stripped
@@ -617,3 +617,33 @@ SELECT 1	latin1	latin1_swedish_ci	latin1
 # Connection default
 DROP USER user2@localhost;
 DROP DATABASE db1;
+#
+# Test for bug#12602983 - User without privilege on routine can discover
+# its existence by executing "select non_existing_func();" or by 
+# "call non_existing_proc()";
+#
+drop database if exists mysqltest_db;
+create database mysqltest_db;
+create function mysqltest_db.f1() returns int return 0;
+create procedure mysqltest_db.p1() begin end;
+# Create user with no privileges on mysqltest_db database.
+create user bug12602983_user@localhost;
+# Connect as user 'bug12602983_user@localhost'
+# Attempt to execute routine on which user doesn't have privileges
+# should result in the same 'access denied' error whether
+# routine exists or not.
+select mysqltest_db.f_does_not_exist();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.f_does_not_exist'
+call mysqltest_db.p_does_not_exist();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.p_does_not_exist'
+select mysqltest_db.f1();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.f1'
+call mysqltest_db.p1();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.p1'
+create view bug12602983_v1 as select mysqltest_db.f_does_not_exist();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.f_does_not_exist'
+create view bug12602983_v1 as select mysqltest_db.f1();
+ERROR 42000: execute command denied to user 'bug12602983_user'@'localhost' for routine 'mysqltest_db.f1'
+# Connection 'default'.
+drop user bug12602983_user@localhost;
+drop database mysqltest_db;

=== modified file 'mysql-test/t/sp-security.test'
--- a/mysql-test/t/sp-security.test	revid:chaithra.gopalareddy@stripped
+++ b/mysql-test/t/sp-security.test	revid:praveenkumar.hulakund@stripped
@@ -995,6 +995,47 @@ disconnect con2;
 DROP USER user2@localhost;
 DROP DATABASE db1;
 
+--echo #
+--echo # Test for bug#12602983 - User without privilege on routine can discover
+--echo # its existence by executing "select non_existing_func();" or by 
+--echo # "call non_existing_proc()";
+--echo #
+--disable_warnings
+drop database if exists mysqltest_db;
+--enable_warnings
+create database mysqltest_db;
+create function mysqltest_db.f1() returns int return 0;
+create procedure mysqltest_db.p1() begin end;
+
+--echo # Create user with no privileges on mysqltest_db database.
+create user bug12602983_user@localhost;
+
+--echo # Connect as user 'bug12602983_user@localhost'
+connect (conn1, localhost, bug12602983_user,,);
+
+--echo # Attempt to execute routine on which user doesn't have privileges
+--echo # should result in the same 'access denied' error whether
+--echo # routine exists or not.
+--error ER_PROCACCESS_DENIED_ERROR
+select mysqltest_db.f_does_not_exist();
+--error ER_PROCACCESS_DENIED_ERROR
+call mysqltest_db.p_does_not_exist();
+
+--error ER_PROCACCESS_DENIED_ERROR
+select mysqltest_db.f1();
+--error ER_PROCACCESS_DENIED_ERROR
+call mysqltest_db.p1();
+
+--error ER_PROCACCESS_DENIED_ERROR
+create view bug12602983_v1 as select mysqltest_db.f_does_not_exist();
+--error ER_PROCACCESS_DENIED_ERROR
+create view bug12602983_v1 as select mysqltest_db.f1();
+
+--echo # Connection 'default'.
+connection default;
+disconnect conn1;
+drop user bug12602983_user@localhost;
+drop database mysqltest_db;
 
 # Wait till all disconnects are completed
 --source include/wait_until_count_sessions.inc

=== modified file 'sql/item_func.cc'
--- a/sql/item_func.cc	revid:chaithra.gopalareddy@stripped
+++ b/sql/item_func.cc	revid:praveenkumar.hulakund@stripped
@@ -6796,9 +6796,42 @@ bool
 Item_func_sp::fix_fields(THD *thd, Item **ref)
 {
   bool res;
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  Security_context *save_security_ctx= thd->security_ctx;
+#endif
+
   DBUG_ENTER("Item_func_sp::fix_fields");
   DBUG_ASSERT(fixed == 0);
- 
+
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  /* 
+    Checking privileges to execute the function while creating view and
+    executing the function of select.
+   */
+  if (!(thd->lex->context_analysis_only & CONTEXT_ANALYSIS_ONLY_VIEW) ||
+      (thd->lex->sql_command == SQLCOM_CREATE_VIEW))
+  {
+    if (context->security_ctx)
+    {
+      /* Set view definer security context */
+      thd->security_ctx= context->security_ctx;
+    }
+
+    /*
+      Check whether user has execute privilege or not
+     */
+    res= check_routine_access(thd, EXECUTE_ACL, m_name->m_db.str,
+                              m_name->m_name.str, 0, FALSE);
+    thd->security_ctx= save_security_ctx;
+
+    if (res)
+    {
+      context->process_error(thd);
+      DBUG_RETURN(res);
+    }
+  }
+#endif
+
   /*
     We must call init_result_field before Item_func::fix_fields() 
     to make m_sp and result_field members available to fix_length_and_dec(),

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	revid:chaithra.gopalareddy@stripped
+++ b/sql/sql_parse.cc	revid:praveenkumar.hulakund@stripped
@@ -4205,6 +4205,13 @@ create_sp_error:
   case SQLCOM_CALL:
     {
       sp_head *sp;
+
+      /* Here we check for the execute privilege on stored procedure. */
+      if (check_routine_access(thd, EXECUTE_ACL, lex->spname->m_db.str,
+                               lex->spname->m_name.str,
+                               lex->sql_command == SQLCOM_CALL, 0))
+        goto error;
+
       /*
         This will cache all SP and SF and open and lock all tables
         required for execution.

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (praveenkumar.hulakund:3801 to 3802)Bug#12602983Praveenkumar Hulakund30 Jan