List:Commits« Previous MessageNext Message »
From:Li-Bing.Song Date:May 12 2010 11:00am
Subject:bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3378) Bug#49124
View as plain text  
#At file:///home/anders/work/bzrwork/worktree2/mysql-5.1-bugteam/ based on revid:mattias.jonsson@stripped

 3378 Li-Bing.Song@stripped	2010-05-12
      BUG#49124 Security issue with /*!-versioned */ SQL statements on Slave
            
      /*!50200 Query Code */ is a special comment that the query in it can
      be executed on those servers whose versions are larger than the 
      version appearing in the comment. It leads to a security issue when 
      slave's version is larger than master's. A malicious user can improve 
      his privileges on slaves. Because slave SQL thread is running with 
      SUPER privileges, so it can execute queries that he/she does not have
      privileges on master.
            
      This bug is fixed with the logic below: 
      For the statements which have at least on version comment not existing on master
      - Strip the whole version comments(include '/*!xxxxxx' and '*/') which
        are not executed on master
      - Strip only '/*!xxxxxx' and '*/', the clause in the comments are kept.
      
        example:        INSERT /*!10000 INTO t1 */ /*!99999 ,t2 */ VALUES(1)
        is binlogged as INSERT INTO t1 VALUES(1)
      
      For others, the original statements are binlogged.
         example:        INSERT /*!10000 INTO t1 */ VALUES(1)
        is binlogged as  INSERT /*!10000 INTO t1 */ VALUES(1)

    added:
      mysql-test/suite/rpl/r/rpl_special_comments.result
      mysql-test/suite/rpl/t/rpl_special_comments.test
    modified:
      sql/sql_lex.cc
      sql/sql_lex.h
      sql/sql_parse.cc
      sql/sql_prepare.cc
=== added file 'mysql-test/suite/rpl/r/rpl_special_comments.result'
--- a/mysql-test/suite/rpl/r/rpl_special_comments.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/r/rpl_special_comments.result	2010-05-12 11:00:36 +0000
@@ -0,0 +1,169 @@
+stop slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+reset master;
+reset slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+start slave;
+
+CREATE TABLE t1(c1 INT);
+
+# Case 1:
+# ----------------------------------------------------------------------
+# Special comments will be executed on master. the comments should be in
+# the log events.
+INSERT INTO /*! t1 */ VALUES(1);
+INSERT INTO t1 VALUES(2), /*!10000 (3)*/;
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO /*! t1 */ VALUES(1)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(2), /*!10000 (3)*/
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 2:
+# --------------------------------------------------------------------
+# Special comments will not be executed on master. the comments should
+# not be in the log events.
+# Comment is in front of the whole statement.
+/*!99999 --- */INSERT INTO t1 VALUES(10);
+# Comment is behind the whole statement.
+INSERT INTO t1 VALUES(11)/*!99999 --- */;
+# Comment is in the middle of the statement.
+INSERT INTO t1 VALUES(12)/*!99999,(13)*/;
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(10)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(11)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(12)
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 3:
+# ------------------------------------------------------------------
+# In a statement, a comment is executed while another is not.
+# All comments should not be in the log events,
+/*!99999 --- */INSERT /*!INTO*/ t1 VALUES(10);
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(10)
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 4:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly, when there are more
+# than one statement.
+/*!10000INSERT INTO t1 VALUES(50)*/; INSERT INTO t1 VALUES(51); INSERT /**/
+INTO t1 VALUES(52)|
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; /*!10000INSERT INTO t1 VALUES(50)*/
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(51)
+master-bin.000001	#	Query	#	#	use `test`; INSERT /**/
+INTO t1 VALUES(52)
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 5:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly when executing prepared
+# statement.
+PREPARE stmt FROM 'INSERT INTO t1 VALUES(60) /*!99999 ,(61)*/';
+EXECUTE stmt;
+DROP TABLE t1;
+CREATE TABLE t1(c1 INT);
+EXECUTE stmt;
+Comparing tables master:test.t1 and slave:test.t1
+
+SET @value=62;
+PREPARE stmt FROM 'INSERT INTO t1 VALUES(?) /*!99999 ,(61)*/';
+EXECUTE stmt USING @value;
+DROP TABLE t1;
+CREATE TABLE t1(c1 INT);
+EXECUTE stmt USING @value;
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(60)
+master-bin.000001	#	Query	#	#	use `test`; DROP TABLE t1
+master-bin.000001	#	Query	#	#	use `test`; CREATE TABLE t1(c1 INT)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(60)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(62)
+master-bin.000001	#	Query	#	#	use `test`; DROP TABLE t1
+master-bin.000001	#	Query	#	#	use `test`; CREATE TABLE t1(c1 INT)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(62)
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 6:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly when creating and  
+# calling a store procedure.
+DROP PROCEDURE IF EXISTS /*!99999 --- */ p1;
+CREATE /*!99999 --- */ PROCEDURE p1() /*!99999 --- */ 
+BEGIN
+INSERT INTO t1 VALUES(70) /*!99999 ,(71)*/;
+INSERT INTO t1 VALUES(72) /*!99999 ,(73)*/;
+END|
+CALL p1;
+CALL p1;
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; DROP PROCEDURE IF EXISTS  p1
+master-bin.000001	#	Query	#	#	use `test`; CREATE DEFINER=`root`@`localhost` PROCEDURE `p1`()
+BEGIN
+INSERT INTO t1 VALUES(70) ;
+INSERT INTO t1 VALUES(72) ;
+END
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(70)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(72)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(70)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(72)
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 7:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly when creating and  
+# calling a store function.
+DROP FUNCTION IF EXISTS f1;
+CREATE FUNCTION /*!99999 --- */ f1() /*!99999 --- */ 
+RETURNS /*!99999 --- */ INT /*!99999 --- */ 
+BEGIN /*!99999 --- */ 
+/*!99999 INSERT INTO t1 VALUES(80);*/
+RETURN  /*!99999 --- */ 81;
+END  /*!99999 --- */ |
+INSERT INTO t1 VALUES(f1());
+INSERT INTO t1 VALUES(f1());
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; DROP FUNCTION IF EXISTS f1
+master-bin.000001	#	Query	#	#	use `test`; CREATE DEFINER=`root`@`localhost` FUNCTION `f1`() RETURNS int(11)
+BEGIN  
+
+RETURN   81;
+END
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(f1())
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(f1())
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 7:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly when create a trigger
+DROP TRIGGER IF EXISTS tr1;
+CREATE TABLE t2(c1 INT);
+CREATE /*!99999 ---*/ /*!TRIGGER*/ tr1 AFTER INSERT ON t1 FOR EACH ROW
+BEGIN/*!99999 ---*/
+/*!99999 UPDATE mysql.user SET Super_priv='Y';*/
+INSERT INTO /*!99999 ---*/ t2 VALUES(NEW.c1);
+END/*!99999 ---*/|
+INSERT INTO t1 VALUES(90), (91);
+INSERT INTO t1 VALUES(92), (93);
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; DROP TRIGGER IF EXISTS tr1
+master-bin.000001	#	Query	#	#	use `test`; CREATE TABLE t2(c1 INT)
+master-bin.000001	#	Query	#	#	use `test`; CREATE DEFINER=`root`@`localhost` TRIGGER tr1 AFTER INSERT ON t1 FOR EACH ROW
+BEGIN
+
+INSERT INTO  t2 VALUES(NEW.c1);
+END
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(90), (91)
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES(92), (93)
+Comparing tables master:test.t2 and slave:test.t2
+DROP TABLE t1, t2;
+DROP PROCEDURE p1;
+DROP FUNCTION f1;

=== added file 'mysql-test/suite/rpl/t/rpl_special_comments.test'
--- a/mysql-test/suite/rpl/t/rpl_special_comments.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/t/rpl_special_comments.test	2010-05-12 11:00:36 +0000
@@ -0,0 +1,207 @@
+###############################################################################
+# After the patch for BUG#49124: 
+# For the statements which have at least on version comment not existing on
+# master 
+# - Strip the whole version comments(include '/*!xxxxxx' and '*/') which are
+# not executed on master 
+# - Strip only '/*!xxxxxx' and '*/', the clause in the comments are kept.
+#
+# example:        INSERT /*!10000 INTO t1 */ /*!99999 ,t2 */ VALUES(1) 
+# is binlogged as INSERT INTO t1 VALUES(1)
+###############################################################################
+source include/master-slave.inc;
+source include/have_binlog_format_statement.inc;
+
+--echo
+CREATE TABLE t1(c1 INT);
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+--echo
+--echo # Case 1:
+--echo # ----------------------------------------------------------------------
+--echo # Special comments will be executed on master. the comments should be in
+--echo # the log events.
+
+INSERT INTO /*! t1 */ VALUES(1);
+
+INSERT INTO t1 VALUES(2), /*!10000 (3)*/;
+
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 2:
+--echo # --------------------------------------------------------------------
+--echo # Special comments will not be executed on master. the comments should
+--echo # not be in the log events.
+
+--echo # Comment is in front of the whole statement.
+/*!99999 --- */INSERT INTO t1 VALUES(10);
+
+--echo # Comment is behind the whole statement.
+INSERT INTO t1 VALUES(11)/*!99999 --- */;
+
+--echo # Comment is in the middle of the statement.
+INSERT INTO t1 VALUES(12)/*!99999,(13)*/;
+
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 3:
+--echo # ------------------------------------------------------------------
+--echo # In a statement, a comment is executed while another is not.
+--echo # All comments should not be in the log events,
+
+/*!99999 --- */INSERT /*!INTO*/ t1 VALUES(10);
+
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 4:
+--echo # -----------------------------------------------------------------
+--echo # Verify whether it can be binlogged correctly, when there are more
+--echo # than one statement.
+DELIMITER |;
+/*!10000INSERT INTO t1 VALUES(50)*/; INSERT INTO t1 VALUES(51); INSERT /**/
+INTO t1 VALUES(52)|
+DELIMITER ;|
+
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 5:
+--echo # -----------------------------------------------------------------
+--echo # Verify whether it can be binlogged correctly when executing prepared
+--echo # statement.
+PREPARE stmt FROM 'INSERT INTO t1 VALUES(60) /*!99999 ,(61)*/';
+EXECUTE stmt;
+DROP TABLE t1;
+CREATE TABLE t1(c1 INT);
+EXECUTE stmt;
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+SET @value=62;
+PREPARE stmt FROM 'INSERT INTO t1 VALUES(?) /*!99999 ,(61)*/';
+EXECUTE stmt USING @value;
+DROP TABLE t1;
+CREATE TABLE t1(c1 INT);
+EXECUTE stmt USING @value;
+
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 6:
+--echo # -----------------------------------------------------------------
+--echo # Verify whether it can be binlogged correctly when creating and  
+--echo # calling a store procedure.
+--disable_warnings
+DROP PROCEDURE IF EXISTS /*!99999 --- */ p1;
+--enable_warnings
+DELIMITER |;
+CREATE /*!99999 --- */ PROCEDURE p1() /*!99999 --- */ 
+BEGIN
+  INSERT INTO t1 VALUES(70) /*!99999 ,(71)*/;
+  INSERT INTO t1 VALUES(72) /*!99999 ,(73)*/;
+END|
+DELIMITER ;|
+CALL p1;
+CALL p1;
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 7:
+--echo # -----------------------------------------------------------------
+--echo # Verify whether it can be binlogged correctly when creating and  
+--echo # calling a store function.
+--disable_warnings
+DROP FUNCTION IF EXISTS f1;
+--enable_warnings
+
+DELIMITER |;
+CREATE FUNCTION /*!99999 --- */ f1() /*!99999 --- */ 
+  RETURNS /*!99999 --- */ INT /*!99999 --- */ 
+BEGIN /*!99999 --- */ 
+  /*!99999 INSERT INTO t1 VALUES(80);*/
+  RETURN  /*!99999 --- */ 81;
+END  /*!99999 --- */ |
+DELIMITER ;|
+
+INSERT INTO t1 VALUES(f1());
+INSERT INTO t1 VALUES(f1());
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t1;
+let $diff_table_2=slave:test.t1;
+source include/diff_tables.inc;
+
+--echo
+--echo # Case 7:
+--echo # -----------------------------------------------------------------
+--echo # Verify whether it can be binlogged correctly when create a trigger
+--disable_warnings
+DROP TRIGGER IF EXISTS tr1;
+--enable_warnings
+CREATE TABLE t2(c1 INT);
+DELIMITER |;
+CREATE /*!99999 ---*/ /*!TRIGGER*/ tr1 AFTER INSERT ON t1 FOR EACH ROW
+BEGIN/*!99999 ---*/
+  /*!99999 UPDATE mysql.user SET Super_priv='Y';*/
+  INSERT INTO /*!99999 ---*/ t2 VALUES(NEW.c1);
+END/*!99999 ---*/|
+DELIMITER ;|
+
+INSERT INTO t1 VALUES(90), (91);
+INSERT INTO t1 VALUES(92), (93);
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+sync_slave_with_master;
+let $diff_table_1=master:test.t2;
+let $diff_table_2=slave:test.t2;
+source include/diff_tables.inc;
+
+
+connection master;
+DROP TABLE t1, t2;
+DROP PROCEDURE p1;
+DROP FUNCTION f1;
+source include/master-slave-end.inc;

=== modified file 'sql/sql_lex.cc'
--- a/sql/sql_lex.cc	2010-04-01 13:15:22 +0000
+++ b/sql/sql_lex.cc	2010-05-12 11:00:36 +0000
@@ -364,6 +364,7 @@ void lex_start(THD *thd)
   lex->server_options.socket= 0;
   lex->server_options.owner= 0;
   lex->server_options.port= -1;
+  lex->found_in_future_version_comment= FALSE;
 
   lex->is_lex_started= TRUE;
   DBUG_VOID_RETURN;
@@ -1312,6 +1313,7 @@ int MYSQLlex(void *arg, void *yythd)
           }
           else
           {
+            lex->found_in_future_version_comment= TRUE;
             comment_closed= ! consume_comment(lip, 1);
             /* version allowed to have one level of comment inside. */
           }

=== modified file 'sql/sql_lex.h'
--- a/sql/sql_lex.h	2010-03-28 08:37:47 +0000
+++ b/sql/sql_lex.h	2010-05-12 11:00:36 +0000
@@ -1771,6 +1771,8 @@ typedef struct st_lex : public Query_tab
   */
   bool protect_against_global_read_lock;
 
+  bool found_in_future_version_comment;
+
   st_lex();
 
   virtual ~st_lex()

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-05-04 14:03:28 +0000
+++ b/sql/sql_parse.cc	2010-05-12 11:00:36 +0000
@@ -5972,7 +5972,11 @@ void mysql_parse(THD *thd, const char *i
             PROCESSLIST.
             Note that we don't need LOCK_thread_count to modify query_length.
           */
-          if (*found_semicolon && (ulong) (*found_semicolon - thd->query()))
+          if (thd->lex->found_in_future_version_comment)
+            thd->set_query_inner((char *)parser_state.m_lip.get_cpp_buf(),
+                                 (uint32) (parser_state.m_lip.get_cpp_ptr() -
+                                           parser_state.m_lip.get_cpp_buf()));
+          else if (*found_semicolon && (ulong)(*found_semicolon - thd->query()))
             thd->set_query_inner(thd->query(),
                                  (uint32) (*found_semicolon -
                                            thd->query() - 1));

=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc	2010-01-16 07:44:24 +0000
+++ b/sql/sql_prepare.cc	2010-05-12 11:00:36 +0000
@@ -3042,6 +3042,11 @@ bool Prepared_statement::prepare(const c
          thd->is_error() ||
          init_param_array(this);
 
+  if (thd->lex->found_in_future_version_comment)
+    thd->set_query_inner((char *)parser_state.m_lip.get_cpp_buf(),
+                         (uint32) (parser_state.m_lip.get_cpp_ptr() -
+                                   parser_state.m_lip.get_cpp_buf()));
+
   lex->set_trg_event_type_for_tables();
 
   /*


Attachment: [text/bzr-bundle] bzr/li-bing.song@sun.com-20100512110036-2mygsl2pivvh2al9.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3378) Bug#49124Li-Bing.Song12 May