List:Commits« Previous MessageNext Message »
From:Libing Song Date:February 26 2010 3:42am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3348)
Bug#49124
View as plain text  
On Thu, 2010-02-25 at 22:01 +0200, Andrei Elkin wrote:
> Li-Bing, hello.
> 
> I am sorry I could not finish review today because of backup backporting escalation.
> Here is one question, what happened to the idea of all-fixes on the master side?
> 
>    >  3343 Li-Bing.Song@stripped	2010-01-30
>    >       BUG#49124 Security issue with /*!-versioned */ SQL statements on Slave
> 
>    >       This bug is fixed with the logic below: 
>    >       - If the clause in a special comment is executed on master, only
>    >         '/*!VERSION' and '*/' are removed, the clause is reserved.  
>    >       - If the clause in a special comment is not executed on master, the
> whole
>    >         comment is removed from binlog query.
> 
> I asked a question about that I thought a missing part and got no reply.
> 
> Let's first clear out the fixing strategy issue in the morning tomorrow.
> 
OK
> cheers,
> 
> Andrei
> 
> > #At file:///home/anders/work/bzrwork/worktree2/mysql-5.1-bugteam/ based on
> revid:bjorn.munch@stripped
> >
> >  3348 Li-Bing.Song@stripped	2010-02-24
> >       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 which's version is 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, even he/her can execute any query on slave. 
> >       Because slave SQL thread is running on super mode, so it can execute 
> >       all queries which are replicated from master.
> >             
> >       This bug is fixed with the logic below: 
> >       master's version is added into query log event as a variable. 
> >       When applying a query log event, the special comments will be
> >       executed only when its version is larger than both of master's
> >       and slave's version. 
> >       For the log events before the patch for BUG#49124, the master's
> >       version is not written into query log event. we see the version in
> >       FORMAT_DESCRIPTION_EVENTit as the master's version.
> >             
> >
> >     added:
> >       mysql-test/std_data/rpl_special_comments.000001
> >       mysql-test/std_data/rpl_special_comments.000002
> >       mysql-test/suite/rpl/r/rpl_special_comments.result
> >       mysql-test/suite/rpl/t/rpl_special_comments.cnf
> >       mysql-test/suite/rpl/t/rpl_special_comments.test
> >     modified:
> >       mysql-test/include/setup_fake_relay_log.inc
> >       sql/log_event.cc
> >       sql/log_event.h
> >       sql/sql_lex.cc
> > === modified file 'mysql-test/include/setup_fake_relay_log.inc'
> > --- a/mysql-test/include/setup_fake_relay_log.inc	2010-02-02 15:16:47 +0000
> > +++ b/mysql-test/include/setup_fake_relay_log.inc	2010-02-24 08:35:29 +0000
> > @@ -77,6 +77,7 @@ if (`SELECT LENGTH(@@secure_file_priv) >
> >    -- let $_tmp_file= $_file_priv_dir/fake-index.$_suffix
> >  
> >    -- eval select '$_fake_filename-fake.000001\n' into dumpfile '$_tmp_file'
> > +  -- remove_file $_fake_relay_index
> >    -- copy_file $_tmp_file $_fake_relay_index
> >    -- remove_file $_tmp_file
> >  }
> >
> > === added file 'mysql-test/std_data/rpl_special_comments.000001'
> > Binary files a/mysql-test/std_data/rpl_special_comments.000001	1970-01-01
> 00:00:00 +0000 and b/mysql-test/std_data/rpl_special_comments.000001	2010-02-24 08:35:29
> +0000 differ
> >
> > === added file 'mysql-test/std_data/rpl_special_comments.000002'
> > Binary files a/mysql-test/std_data/rpl_special_comments.000002	1970-01-01
> 00:00:00 +0000 and b/mysql-test/std_data/rpl_special_comments.000002	2010-02-24 08:35:29
> +0000 differ
> >
> > === 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-02-24 08:35:29
> +0000
> > @@ -0,0 +1,88 @@
> > +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;
> > +CHANGE MASTER TO master_host='127.0.0.1', master_port=SLAVE_MYPORT,
> > +master_log_file='slave-bin.000001', master_user='root';
> > +START SLAVE;
> > +
> > +# Case 1:
> > +# The special comments are not executed both on master an slave.
> > +# ----------------------------------------------------------------------
> > +CREATE /*!99999 xyz */ TABLE t1(c1 INT);
> > +
> > +# Case 2:
> > +# The comment always is executed on master and slave
> > +# ----------------------------------------------------------------------
> > +CREATE /*! TABLE */ t2(c1 INT);
> > +
> > +# Case 3:
> > +# The comment which is executed on master also is executed on slave if
> > +# its version is equal to or large than the version of the comment.
> > +# ----------------------------------------------------------------------
> > +INSERT /*!20000 INTO */ t2 VALUES(1);
> > +
> > +# Case 4:
> > +# Though slave's version is larger than the version of the special
> > +# comments in the following queries, the special comments are not
> > +# executed on the slave. For their version is larger than master's version
> > +# which is in FORMAT_DESCRIPTION_EVENT.
> > +# ----------------------------------------------------------------------
> > +# [on slave]
> > +STOP SLAVE;
> > +RESET SLAVE;
> > +FLUSH LOGS;
> > +# rpl_special_comments.000001 is generated by MySQL 5.00.86
> > +Setting up fake replication from
> MYSQL_TEST_DIR/std_data/rpl_special_comments.000001
> > +START SLAVE SQL_THREAD;
> > +STOP SLAVE SQL_THREAD;
> > +Cleaning up after setup_fake_relay_log.inc
> > +SELECT * FROM t1;
> > +c1
> > +2
> > +SELECT * FROM t2;
> > +c1
> > +1
> > +# [on slave2]
> > +SELECT * FROM t1;
> > +c1
> > +2
> > +SELECT * FROM t2;
> > +c1
> > +1
> > +
> > +# Case 5:
> > +# Though master's version is larger than the version of the special
> > +# comments in the following queries, the special comments are not
> > +# executed on the slave. For their version is larger than slave's version.
> > +# /*!99999 DROP TABLE t1; -- */ INSERT INTO t1 VALUES(1)
> > +# ----------------------------------------------------------------------
> > +# [on slave]
> > +RESET SLAVE;
> > +Warnings:
> > +Warning	1612	Being purged log slave-relay-bin-fake.000001 was not found
> > +Warning	1612	Being purged log ./slave-relay-bin.index was not found
> > +TRUNCATE t1;
> > +FLUSH LOGS;
> > +# rpl_special_comments.000002 is a faked binlog file which's server version is
> 99999.
> > +Setting up fake replication from
> MYSQL_TEST_DIR/std_data/rpl_special_comments.000002
> > +START SLAVE SQL_THREAD;
> > +STOP SLAVE SQL_THREAD;
> > +Cleaning up after setup_fake_relay_log.inc
> > +SELECT * FROM t1;
> > +c1
> > +2
> > +SELECT * FROM t2;
> > +c1
> > +1
> > +# [on slave2]
> > +SELECT * FROM t1;
> > +c1
> > +2
> > +SELECT * FROM t2;
> > +c1
> > +1
> > +DROP TABLE t1, t2;
> > +DROP TABLE t1, t2;
> >
> > === added file 'mysql-test/suite/rpl/t/rpl_special_comments.cnf'
> > --- a/mysql-test/suite/rpl/t/rpl_special_comments.cnf	1970-01-01 00:00:00 +0000
> > +++ b/mysql-test/suite/rpl/t/rpl_special_comments.cnf	2010-02-24 08:35:29 +0000
> > @@ -0,0 +1,13 @@
> > +!include ../my.cnf
> > +
> > +[mysqld.2]
> > +init-rpl-role=master
> > +
> > +[mysqld.3]
> > +init-rpl-role=slave
> > +log-bin=slave-bin
> > +
> > +
> > +[ENV]
> > +SLAVE_MYPORT1=		@mysqld.3.port
> > +SLAVE_MYSOCK1=		@mysqld.3.socket
> >
> > === 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-02-24 08:35:29
> +0000
> > @@ -0,0 +1,125 @@
> >
> +###############################################################################
> > +# Bug#49124 Security issue with /*!-versioned */ SQL statements on Slave
> > +#
> > +# Though the special comment which's version is larger than the master's
> > +# version is not executed on master, it is still binlogged and then it is
> > +# executed on slave if slave's version is equal to or larger than its version.
> > +# This can be used by a malicious user to improve privileges on slaves.
> > +#
> > +# After this patch, master's version is added into query log event as
> > +# a variable. When applying a query log event, the special comments will be
> > +# executed only when its version is larger than both of master's and slave's
> > +# version. For the log events before the patch for BUG#49124, the master's
> > +# version is not written into query log event. we see the version in
> > +# FORMAT_DESCRIPTION_EVENTit as the master's version.
> > +#
> ###############################################################################
> > +source include/master-slave.inc;
> > +source include/have_binlog_format_statement.inc;
> > +
> > +connect (slave2,127.0.0.1,root,,test,$SLAVE_MYPORT1,);
> > +connection slave2;
> > +--replace_result $SLAVE_MYPORT SLAVE_MYPORT
> > +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SLAVE_MYPORT,
> > +     master_log_file='slave-bin.000001', master_user='root';
> > +START SLAVE;
> > +source include/wait_for_slave_to_start.inc;
> > +
> > +connection master;
> > +--echo
> > +--echo # Case 1:
> > +--echo # The special comments are not executed both on master an slave.
> > +--echo #
> ----------------------------------------------------------------------
> > +CREATE /*!99999 xyz */ TABLE t1(c1 INT);
> > +
> > +--echo
> > +--echo # Case 2:
> > +--echo # The comment always is executed on master and slave
> > +--echo #
> ----------------------------------------------------------------------
> > +CREATE /*! TABLE */ t2(c1 INT);
> > +
> > +--echo
> > +--echo # Case 3:
> > +--echo # The comment which is executed on master also is executed on slave if
> > +--echo # its version is equal to or large than the version of the comment.
> > +--echo #
> ----------------------------------------------------------------------
> > +INSERT /*!20000 INTO */ t2 VALUES(1);
> > +sync_slave_with_master;
> > +
> > +--echo
> > +--echo # Case 4:
> > +--echo # Though slave's version is larger than the version of the special
> > +--echo # comments in the following queries, the special comments are not
> > +--echo # executed on the slave. For their version is larger than the master's
> > +--echo # version in FORMAT_DESCRIPTION_EVENT.
> > +--echo #
> > +--echo # But there is a flaw in this solution. In the cascade replication,
> > +--echo # for example: A ---> B ---> C.  A is 50086, B and C is 50144
> > +--echo # B will write its own version into the FORMAT_DESCRIPTION_EVENT. So
> > +--echo # C will apply the special comments.
> > +--echo #
> ----------------------------------------------------------------------
> > +--echo # [on slave]
> > +STOP SLAVE;
> > +source include/wait_for_slave_to_stop.inc;
> > +RESET SLAVE;
> > +
> > +--echo # rpl_special_comments.000001 is generated by MySQL 5.00.86
> > +# The following queries are in the binlog file 
> > +# /*!50087 DROP TABLE t1; -- */ INSERT INTO t1 VALUES(1) 
> > +# UPDATE t1 /*!50087 , t2 */ SET t1.c1=2 /*!50087 , t2.c1=2*/;
> > +#copy_file $MYSQLTEST_VARDIR/std_data/rpl_special_comments.000001
> $MYSQLD_DATADIR/master-bin.000002;
> > +let $fake_relay_log= $MYSQL_TEST_DIR/std_data/rpl_special_comments.000001;
> > +source include/setup_fake_relay_log.inc;
> > +START SLAVE SQL_THREAD;
> > +let $slave_param = Exec_Master_Log_Pos;
> > +# end_log_pos of the last event of the relay log
> > +let $slave_param_value = 337;
> > +source include/wait_for_slave_param.inc;
> > +STOP SLAVE SQL_THREAD;
> > +source include/wait_for_slave_sql_to_stop.inc;
> > +source include/cleanup_fake_relay_log.inc;
> > +
> > +SELECT * FROM t1;
> > +SELECT * FROM t2;
> > +sync_slave_with_master slave2;
> > +--echo # [on slave2]
> > +SELECT * FROM t1;
> > +SELECT * FROM t2;
> > +
> > +--echo
> > +--echo # Case 5:
> > +--echo # Though master's version is larger than the version of the special
> > +--echo # comments in the following queries, the special comments are not
> > +--echo # executed on the slave. For their version is larger than slave's
> version.
> > +--echo # /*!99999 DROP TABLE t1; -- */ INSERT INTO t1 VALUES(1)
> > +--echo #
> ----------------------------------------------------------------------
> > +connection slave;
> > +--echo # [on slave]
> > +RESET SLAVE;
> > +TRUNCATE t1;
> > +--echo # rpl_special_comments.000002 is a faked binlog file which's server
> version is 99999.
> > +# The following queries are in the binlog file 
> > +# /*!99999 DROP TABLE t1; -- */ INSERT INTO t1 VALUES(1) 
> > +# UPDATE t1 /*!99999 , t2 */ SET t1.c1=2 /*!99999 , t2.c1=2*/;
> > +let $fake_relay_log= $MYSQL_TEST_DIR/std_data/rpl_special_comments.000002;
> > +source include/setup_fake_relay_log.inc;
> > +START SLAVE SQL_THREAD;
> > +let $slave_param = Exec_Master_Log_Pos;
> > +# end_log_pos of the last event of the relay log
> > +let $slave_param_value = 337;
> > +--source include/wait_for_slave_param.inc
> > +STOP SLAVE SQL_THREAD;
> > +source include/wait_for_slave_sql_to_stop.inc;
> > +source include/cleanup_fake_relay_log.inc;
> > +
> > +SELECT * FROM t1;
> > +SELECT * FROM t2;
> > +sync_slave_with_master slave2;
> > +--echo # [on slave2]
> > +SELECT * FROM t1;
> > +SELECT * FROM t2;
> > +
> > +connection master;
> > +DROP TABLE t1, t2;
> > +connection slave;
> > +DROP TABLE t1, t2;
> > +sync_slave_with_master slave2;
> >
> > === modified file 'sql/log_event.cc'
> > --- a/sql/log_event.cc	2010-02-05 17:48:01 +0000
> > +++ b/sql/log_event.cc	2010-02-24 08:35:29 +0000
> > @@ -2307,6 +2307,21 @@ bool Query_log_event::write(IO_CACHE* fi
> >      start+= 4;
> >    }
> >  
> > +  /* Only the first master's version is written into Query_log_event. */
> > +  *start++= Q_MASTER_VERSION;
> > +#ifdef HAVE_REPLICATION
> > +  if (thd->slave_thread)
> > +  {
> > +    Format_description_log_event *des_ev=
> > +      active_mi->rli.relay_log.description_event_for_exec;
> > +    int4store(start, des_ev->get_master_version());
> > +  }
> > +  else
> > +#endif
> > +  {
> > +    int4store(start, MYSQL_VERSION_ID);
> > +  }
> > +  start+= 4;
> >    /*
> >      NOTE: When adding new status vars, please don't forget to update
> >      the MAX_SIZE_LOG_EVENT_STATUS in log_event.h and update the function
> > @@ -2549,7 +2564,7 @@ Query_log_event::Query_log_event(const c
> >     flags2_inited(0), sql_mode_inited(0), charset_inited(0),
> >     auto_increment_increment(1), auto_increment_offset(1),
> >     time_zone_len(0), lc_time_names_number(0), charset_database_number(0),
> > -   table_map_for_update(0), master_data_written(0)
> > +   table_map_for_update(0), master_data_written(0), master_version(0)
> >  {
> >    ulong data_len;
> >    uint32 tmp;
> > @@ -2713,6 +2728,10 @@ Query_log_event::Query_log_event(const c
> >        data_written= master_data_written= uint4korr(pos);
> >        pos+= 4;
> >        break;
> > +    case Q_MASTER_VERSION:
> > +      CHECK_SPACE(pos, end, 4);
> > +      master_version= uint4korr(pos);
> > +      pos+= 4;
> >      default:
> >        /* That's why you must write status vars in growing order of code */
> >        DBUG_PRINT("info",("Query_log_event has unknown status vars (first has\
> > @@ -3163,6 +3182,15 @@ int Query_log_event::do_apply_event(Rela
> >        }
> >        else
> >          thd->variables.collation_database= thd->db_charset;
> > +
> > +      uint32 old_master_version= 0;
> > +      Format_description_log_event *des_ev=
> > +        rli->relay_log.description_event_for_exec;
> > +      if (master_version > 0)
> > +      {
> > +        old_master_version= des_ev->get_master_version();
> > +        des_ev->set_master_version(master_version);
> > +      }
> >        
> >        thd->table_map_for_update= (table_map)table_map_for_update;
> >        
> > @@ -3170,6 +3198,8 @@ int Query_log_event::do_apply_event(Rela
> >        const char* found_semicolon= NULL;
> >        mysql_parse(thd, thd->query(), thd->query_length(),
> &found_semicolon);
> >        log_slow_statement(thd);
> > +      if (old_master_version > 0)
> > +        des_ev->set_master_version(old_master_version);
> >  
> >        /*
> >          Resetting the enable_slow_log thd variable.
> > @@ -4026,6 +4056,10 @@ void Format_description_log_event::calc_
> >      if (*r == '.')
> >        p++; // skip the dot
> >    }
> > +  master_version= (ulong)server_version_split[0] * 10000 +
> > +    (ulong)server_version_split[1] * 100 + (ulong)server_version_split[2] ;
> > +  DBUG_PRINT("info", ("Format_description_log_event::master_version:"
> > +                      " %d", master_version));
> >    DBUG_PRINT("info",("Format_description_log_event::server_version_split:"
> >                       " '%s' %d %d %d", server_version,
> >                       server_version_split[0],
> >
> > === modified file 'sql/log_event.h'
> > --- a/sql/log_event.h	2010-01-27 17:27:49 +0000
> > +++ b/sql/log_event.h	2010-02-24 08:35:29 +0000
> > @@ -264,7 +264,8 @@ struct sql_ex_info
> >                                     1 + 2          /* type, lc_time_names_number
> */ + \
> >                                     1 + 2          /* type,
> charset_database_number */ + \
> >                                     1 + 8          /* type, table_map_for_update
> */ + \
> > -                                   1 + 4          /* type, master_data_written
> */)
> > +                                   1 + 4          /* type, master_data_written
> */ + \
> > +                                   1 + 4          /* type, master_version*/)
> >  #define MAX_LOG_EVENT_HEADER   ( /* in order of Query_log_event::write */ \
> >    LOG_EVENT_HEADER_LEN + /* write_header */ \
> >    QUERY_HEADER_LEN     + /* write_data */   \
> > @@ -333,6 +334,8 @@ struct sql_ex_info
> >  
> >  #define Q_MASTER_DATA_WRITTEN_CODE 10
> >  
> > +#define Q_MASTER_VERSION           11
> > +
> >  /* Intvar event post-header */
> >  
> >  /* Intvar event data */
> > @@ -1190,7 +1193,6 @@ protected:
> >  #endif
> >  };
> >  
> > -
> >  /*
> >     One class for each type of event.
> >     Two constructors for each class:
> > @@ -1636,6 +1638,23 @@ public:
> >    */
> >    uint32 master_data_written;
> >  
> > +  /*
> > +    This master version is extracted from Query_log_event.
> > +    When binlogging, server's version is written into the var part of
> > +    Query_log_event.
> > +
> > +    In cascade replication, master_version always keep the verison of original
> > +    master which really creates the Query_log_event.
> > +
> > +    For example:
> > +    A------>B------>C
> > +    A'version 50001
> > +    B'version 50002
> > +    Whening applying the Query_log_event, Both B and C know that
> master_version
> > +    is 50001.
> > +   */
> > +  uint32 master_version;
> > +
> >  #ifndef MYSQL_CLIENT
> >  
> >    Query_log_event(THD* thd_arg, const char* query_arg, ulong query_length,
> > @@ -2180,6 +2199,7 @@ protected:
> >  
> >  class Format_description_log_event: public Start_log_event_v3
> >  {
> > +  uint32 master_version;
> >  public:
> >    /*
> >       The size of the fixed header which _all_ events have
> > @@ -2223,6 +2243,8 @@ public:
> >    }
> >  
> >    void calc_server_version_split();
> > +  uint32 get_master_version() { return master_version; }
> > +  void set_master_version(uint32 version) { master_version= version; }
> >  
> >  protected:
> >  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
> >
> > === modified file 'sql/sql_lex.cc'
> > --- a/sql/sql_lex.cc	2010-02-06 19:54:30 +0000
> > +++ b/sql/sql_lex.cc	2010-02-24 08:35:29 +0000
> > @@ -23,6 +23,9 @@
> >  #include <hash.h>
> >  #include "sp.h"
> >  #include "sp_head.h"
> > +#ifdef HAVE_REPLICATION
> > +#include "rpl_mi.h"
> > +#endif
> >  
> >  /*
> >    We are using pointer to this variable for distinguishing between assignment
> > @@ -1303,7 +1306,22 @@ int MYSQLlex(void *arg, void *yythd)
> >            /* Accept 'M' 'm' 'm' 'd' 'd' */
> >            lip->yySkipn(5);
> >  
> > -          if (version <= MYSQL_VERSION_ID)
> > +          if (version <= MYSQL_VERSION_ID
> > +#ifdef HAVE_REPLICATION
> > +              /*
> > +                Bug#49124 Security issue with / *!-versioned * / SQL statements
> on
> > +                Slave
> > +
> > +                The bug is fixed with the logic below:
> > +                Even though the slave's version is larger than the spcial
> comments'
> > +                version, they are not executed on slave if their versions are
> larger
> > +                than master's version which appears in
> FORMAT_DESCRIPTION_EVENT.
> > +               */
> > +              &&
> > +              (!thd->slave_thread || version <=
> > +              
> active_mi->rli.relay_log.description_event_for_exec->get_master_version())
> > +#endif
> > +             )
> >            {
> >              /* Expand the content of the special comment as real code */
> >              lip->set_echo(TRUE);
> >
> >


-- 
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer
Certified (ISC)2 CISSP

Email : Li-Bing.Song@stripped
Skype : libing.song
MSN   : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================


Thread
bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3348) Bug#49124Li-Bing.Song24 Feb
  • Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3348)Bug#49124He Zhenxing4 Mar
Re: bzr commit into mysql-5.1-bugteam branch (Li-Bing.Song:3348)Bug#49124Libing Song26 Feb