List:Commits« Previous MessageNext Message »
From:Li-Bing.Song Date:July 29 2010 3:01am
Subject:bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3459) Bug#49124
View as plain text  
#At file:///home/anders/work/bzrwork1/wt1/mysql-5.1-bugteam/ based on revid:davi.arnaut@stripped

 3459 Li-Bing.Song@stripped	2010-07-29
      BUG#49124 Security issue with /*!-versioned */ SQL statements on Slave
      
      /*![:version:] Query Code */, where [:version:] is a sequence of 5 
      digits representing the mysql server version(e.g /*!50200 ... */),
      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: 
      - To replace '!' with ' ' in the magic comments which are not applied on
        master. So they become common comments and will not be applied on slave.
      
      - Example:
        'INSERT INTO t1 VALUES (1) /*!10000, (2)*/ /*!99999 ,(3)*/
        will be binlogged as
        'INSERT INTO t1 VALUES (1) /*!10000, (2)*/ /* 99999 ,(3)*/
     @ mysql-test/suite/rpl/t/rpl_conditional_comments.test
        Test the patch for this bug.
     @ sql/mysql_priv.h
        Rename inBuf as rawBuf and remove the const limitation.
     @ sql/sql_lex.cc
        To replace '!' with ' ' in the magic comments which are not applied on
        master.
     @ sql/sql_lex.h
        Remove the const limitation on parameter buff, as it can be modified in the function since
        this patch.
        Add member function yyUnput for Lex_input_stream. It set a character back the query buff.
     @ sql/sql_parse.cc
        Rename inBuf as rawBuf and remove the const limitation.
     @ sql/sql_partition.cc
        Remove the const limitation on parameter part_buff, as it can be modified in the function since
        this patch.
     @ sql/sql_partition.h
        Remove the const limitation on parameter part_buff, as it can be modified in the function since
        this patch.
     @ sql/table.h
        Remove the const limitation on variable partition_info, as it can be modified since
        this patch.

    added:
      mysql-test/suite/rpl/r/rpl_conditional_comments.result
      mysql-test/suite/rpl/t/rpl_conditional_comments.test
    modified:
      sql/mysql_priv.h
      sql/sql_lex.cc
      sql/sql_lex.h
      sql/sql_parse.cc
      sql/sql_partition.cc
      sql/sql_partition.h
      sql/table.h
=== added file 'mysql-test/suite/rpl/r/rpl_conditional_comments.result'
--- a/mysql-test/suite/rpl/r/rpl_conditional_comments.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/r/rpl_conditional_comments.result	2010-07-29 03:00:57 +0000
@@ -0,0 +1,57 @@
+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);
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; CREATE TABLE t1(c1 INT)
+
+# Case 1:
+# ------------------------------------------------------------------
+# In a statement, some CCs are applied while others are not. The CCs
+# which are not applied on master will be binlogged as common comments.
+/*!99999 --- */INSERT /*!INTO*/ /*!10000 t1 */ VALUES(10) /*!99999 ,(11)*/;
+show binlog events from <binlog_start>;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	use `test`; /* 99999 --- */INSERT /*!INTO*/ /*!10000 t1 */ VALUES(10) /* 99999 ,(11)*/
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 2:
+# -----------------------------------------------------------------
+# Verify whether it can be binlogged correctly when executing prepared
+# statement.
+PREPARE stmt FROM 'INSERT INTO /*!99999 blabla*/ 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 /*!99999 blabla */ t1 VALUES(?) /*!99999 ,(63)*/';
+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 /* 99999 blabla*/ t1 VALUES(60) /* 99999 ,(61)*/
+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 /* 99999 blabla*/ t1 VALUES(60) /* 99999 ,(61)*/
+master-bin.000001	#	Query	#	#	use `test`; INSERT INTO /* 99999 blabla */ t1 VALUES(62) /* 99999 ,(63)*/
+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 /* 99999 blabla */ t1 VALUES(62) /* 99999 ,(63)*/
+Comparing tables master:test.t1 and slave:test.t1
+
+# Case 3:
+# -----------------------------------------------------------------
+# Verify it can restore the '!', if the it is an uncomplete conditional
+# comments
+SELECT c1 FROM /*!99999 t1 WHEREN;
+ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '/*!99999 t1 WHEREN' at line 1
+DROP TABLE t1;

=== added file 'mysql-test/suite/rpl/t/rpl_conditional_comments.test'
--- a/mysql-test/suite/rpl/t/rpl_conditional_comments.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/t/rpl_conditional_comments.test	2010-07-29 03:00:57 +0000
@@ -0,0 +1,74 @@
+###############################################################################
+# After the patch for BUG#49124:
+#  - Use ' ' instead of '!' in the conditional comments which are not applied on
+#  master. So they become common comments and will not be applied on slave.
+#
+#  - Example: 
+#  'INSERT INTO t1 VALUES (1) /*!10000, (2)*/ /*!99999 ,(3)*/ 
+#  will be binlogged as 
+#  'INSERT INTO t1 VALUES (1) /*!10000, (2)*/ /* 99999 ,(3)*/'.
+###############################################################################
+source include/master-slave.inc;
+source include/have_binlog_format_statement.inc;
+
+CREATE TABLE t1(c1 INT);
+source include/show_binlog_events.inc;
+let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
+
+--echo
+--echo # Case 1:
+--echo # ------------------------------------------------------------------
+--echo # In a statement, some CCs are applied while others are not. The CCs
+--echo # which are not applied on master will be binlogged as common comments.
+
+/*!99999 --- */INSERT /*!INTO*/ /*!10000 t1 */ VALUES(10) /*!99999 ,(11)*/;
+
+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 # Verify whether it can be binlogged correctly when executing prepared
+--echo # statement.
+PREPARE stmt FROM 'INSERT INTO /*!99999 blabla*/ 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 /*!99999 blabla */ t1 VALUES(?) /*!99999 ,(63)*/';
+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 3:
+--echo # -----------------------------------------------------------------
+--echo # Verify it can restore the '!', if the it is an uncomplete conditional
+--echo # comments
+--error 1064
+SELECT c1 FROM /*!99999 t1 WHEREN;
+
+DROP TABLE t1;
+source include/master-slave-end.inc;

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2010-07-19 14:30:34 +0000
+++ b/sql/mysql_priv.h	2010-07-29 03:00:57 +0000
@@ -1024,7 +1024,7 @@ bool mysql_opt_change_db(THD *thd,
                          bool force_switch,
                          bool *cur_db_changed);
 
-void mysql_parse(THD *thd, const char *inBuf, uint length,
+void mysql_parse(THD *thd, char *rawbuf, uint length,
                  const char ** semicolon);
 
 bool mysql_test_parse_for_slave(THD *thd,char *inBuf,uint length);

=== modified file 'sql/sql_lex.cc'
--- a/sql/sql_lex.cc	2010-07-19 14:30:34 +0000
+++ b/sql/sql_lex.cc	2010-07-29 03:00:57 +0000
@@ -111,7 +111,7 @@ st_parsing_options::reset()
 }
 
 
-bool Lex_input_stream::init(THD *thd, const char *buff, unsigned int length)
+bool Lex_input_stream::init(THD *thd, char *buff, unsigned int length)
 {
   DBUG_EXECUTE_IF("bug42064_simulate_oom",
                   DBUG_SET("+d,simulate_out_of_memory"););
@@ -1292,11 +1292,10 @@ int MYSQLlex(void *arg, void *yythd)
           ulong version;
           version=strtol(version_str, NULL, 10);
 
-          /* Accept 'M' 'm' 'm' 'd' 'd' */
-          lip->yySkipn(5);
-
           if (version <= MYSQL_VERSION_ID)
           {
+            /* Accept 'M' 'm' 'm' 'd' 'd' */
+            lip->yySkipn(5);
             /* Expand the content of the special comment as real code */
             lip->set_echo(TRUE);
             state=MY_LEX_START;
@@ -1304,7 +1303,19 @@ int MYSQLlex(void *arg, void *yythd)
           }
           else
           {
+            const char* version_mark= lip->get_ptr() - 1;
+            DBUG_ASSERT(*version_mark == '!');
+            /*
+              Patch and skip the conditional comment to avoid it
+              being propagated infinitely (eg. to a slave).
+            */
+            char *pcom= lip->yyUnput(' ');
             comment_closed= ! consume_comment(lip, 1);
+            if (! comment_closed)
+            {
+              DBUG_ASSERT(pcom == version_mark);
+              *pcom= '!';
+            }
             /* version allowed to have one level of comment inside. */
           }
         }

=== modified file 'sql/sql_lex.h'
--- a/sql/sql_lex.h	2010-06-11 12:52:06 +0000
+++ b/sql/sql_lex.h	2010-07-29 03:00:57 +0000
@@ -1180,7 +1180,7 @@ public:
      @retval FALSE OK
      @retval TRUE  Error
   */
-  bool init(THD *thd, const char *buff, unsigned int length);
+  bool init(THD *thd, char *buff, unsigned int length);
   /**
     Set the echo mode.
 
@@ -1295,6 +1295,20 @@ public:
   }
 
   /**
+    Puts a character back into the stream, canceling
+    the effect of the last yyGet() or yySkip().
+    Note that the echo mode should not change between calls
+    to unput, get, or skip from the stream.
+  */
+  char *yyUnput(char ch)
+  {
+    *--m_ptr= ch;
+    if (m_echo)
+      m_cpp_ptr--;
+    return m_ptr;
+  }
+
+  /**
     End of file indicator for the query text to parse.
     @return true if there are no more characters to parse
   */
@@ -1440,7 +1454,7 @@ public:
 
 private:
   /** Pointer to the current position in the raw input stream. */
-  const char *m_ptr;
+  char *m_ptr;
 
   /** Starting position of the last token parsed, in the raw buffer. */
   const char *m_tok_start;
@@ -1972,7 +1986,7 @@ public:
      @retval FALSE OK
      @retval TRUE  Error
   */
-  bool init(THD *thd, const char *buff, unsigned int length)
+  bool init(THD *thd, char *buff, unsigned int length)
   {
     return m_lip.init(thd, buff, length);
   }

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-07-23 11:15:56 +0000
+++ b/sql/sql_parse.cc	2010-07-29 03:00:57 +0000
@@ -5946,13 +5946,13 @@ void mysql_init_multi_delete(LEX *lex)
   Parse a query.
 
   @param       thd     Current thread
-  @param       inBuf   Begining of the query text
+  @param       rawbuf  Begining of the query text
   @param       length  Length of the query text
   @param[out]  found_semicolon For multi queries, position of the character of
                                the next query in the query text.
 */
 
-void mysql_parse(THD *thd, const char *inBuf, uint length,
+void mysql_parse(THD *thd, char *rawbuf, uint length,
                  const char ** found_semicolon)
 {
   DBUG_ENTER("mysql_parse");
@@ -5978,7 +5978,7 @@ void mysql_parse(THD *thd, const char *i
   lex_start(thd);
   mysql_reset_thd_for_next_command(thd);
 
-  if (query_cache_send_result_to_client(thd, (char*) inBuf, length) <= 0)
+  if (query_cache_send_result_to_client(thd, rawbuf, length) <= 0)
   {
     LEX *lex= thd->lex;
 
@@ -5987,7 +5987,7 @@ void mysql_parse(THD *thd, const char *i
 
     Parser_state parser_state;
     bool err;
-    if (!(err= parser_state.init(thd, inBuf, length)))
+    if (!(err= parser_state.init(thd, rawbuf, length)))
     {
       err= parse_sql(thd, & parser_state, NULL);
       *found_semicolon= parser_state.m_lip.found_semicolon;
@@ -6073,14 +6073,14 @@ void mysql_parse(THD *thd, const char *i
     1	can be ignored
 */
 
-bool mysql_test_parse_for_slave(THD *thd, char *inBuf, uint length)
+bool mysql_test_parse_for_slave(THD *thd, char *rawbuf, uint length)
 {
   LEX *lex= thd->lex;
   bool error= 0;
   DBUG_ENTER("mysql_test_parse_for_slave");
 
   Parser_state parser_state;
-  if (!(error= parser_state.init(thd, inBuf, length)))
+  if (!(error= parser_state.init(thd, rawbuf, length)))
   {
     lex_start(thd);
     mysql_reset_thd_for_next_command(thd);

=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc	2010-05-21 11:23:48 +0000
+++ b/sql/sql_partition.cc	2010-07-29 03:00:57 +0000
@@ -3876,7 +3876,7 @@ void get_partition_set(const TABLE *tabl
 */
 
 bool mysql_unpack_partition(THD *thd,
-                            const char *part_buf, uint part_info_len,
+                            char *part_buf, uint part_info_len,
                             const char *part_state, uint part_state_len,
                             TABLE* table, bool is_create_table_ind,
                             handlerton *default_db_type,

=== modified file 'sql/sql_partition.h'
--- a/sql/sql_partition.h	2009-12-13 20:29:50 +0000
+++ b/sql/sql_partition.h	2010-07-29 03:00:57 +0000
@@ -78,7 +78,7 @@ void get_full_part_id_from_key(const TAB
                                KEY *key_info,
                                const key_range *key_spec,
                                part_id_range *part_spec);
-bool mysql_unpack_partition(THD *thd, const char *part_buf,
+bool mysql_unpack_partition(THD *thd, char *part_buf,
                             uint part_info_len,
                             const char *part_state, uint part_state_len,
                             TABLE *table, bool is_create_table_ind,

=== modified file 'sql/table.h'
--- a/sql/table.h	2010-06-10 20:45:22 +0000
+++ b/sql/table.h	2010-07-29 03:00:57 +0000
@@ -442,7 +442,7 @@ typedef struct st_table_share
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   /** @todo: Move into *ha_data for partitioning */
   bool auto_partitioned;
-  const char *partition_info;
+  char *partition_info;
   uint  partition_info_len;
   uint  partition_info_buffer_size;
   const char *part_state;


Attachment: [text/bzr-bundle] bzr/li-bing.song@sun.com-20100729030057-83w93j2u5dsna1c9.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3459) Bug#49124Li-Bing.Song29 Jul