Uploaded image for project: 'Percona Server for MySQL'
  1. Percona Server for MySQL
  2. PS-4791

SQL injection on slave due to non-quoting in binlogged ROLLBACK TO SAVEPOINT

Details

    Description

      Copy of https://bugs.mysql.com/bug.php?id=92227:

      [29 Aug 12:17] Laurynas Biveinis
      Description:
      This is CVE-2012-4414, bug 66550, bug 68045, bug 69124 once again. Fixes for these bugs tried to ensure that, whenever an identifier is written to binary log, it is properly quoted to prevent any SQL injection on the slave. For binlogging of ROLLBACK TO SAVEPOINT, this was correctly fixed on 5.5 [1]:

      @@ -1731,18 +1739,23 @@ static int binlog_savepoint_rollback(handlerton *hton, THD *thd, void *sv)
      non-transactional table. Otherwise, truncate the binlog cache starting
      from the SAVEPOINT command.
      */

      • if (unlikely(trans_has_updated_non_trans_table(thd) ||
        + if (unlikely(trans_has_updated_non_trans_table(thd) ||
        (thd->options & OPTION_KEEP_LOG)))
        {
      • String log_query;
      • if (log_query.append(STRING_WITH_LEN("ROLLBACK TO ")) ||
      • log_query.append("`") ||
      • log_query.append(thd->lex->ident.str, thd->lex->ident.length) ||
      • log_query.append("`"))
        + // buffer to store rollback query with quoted identifier
        + char* buffer= (char *)my_malloc(12 + 1 + NAME_LEN * 2 + 2, MYF(0));
        + String log_query(buffer, sizeof(buffer), system_charset_info);
        + log_query.length(0);
        +
        + if (log_query.append(STRING_WITH_LEN("ROLLBACK TO ")))
        DBUG_RETURN(1);
        + else
        + append_identifier(thd, &log_query, thd->lex->ident.str,
        + thd->lex->ident.length);

      but something went wrong with 5.6 (5.7, 8.0...) merge, where it still features plain "log_query.append(thd->lex->ident.str, thd->lex->ident.length)" instead of append_identifier call. Consequently unquoted identifiers appear on the master binlog.

      [1]:

      commit 5f003eca000167edc3601168029a7d86468e52a8
      Author: Rohit Kalhans <[email protected]>
      Date: Sat Sep 22 17:50:51 2012 +0530

      BUG#14548159: NUMEROUS CASES OF INCORRECT IDENTIFIER
      QUOTING IN REPLICATION

      Problem: Misquoting or unquoted identifiers may lead to
      incorrect statements to be logged to the binary log.

      Fix: we use specialized functions to append quoted identifiers in
      the statements generated by the server.

      How to repeat:
      --source include/have_binlog_format_statement.inc
      --source include/master-slave.inc

      connection master;

      create table t1 (a int) engine=innodb;
      create table t2 (a int) engine=myisam;

      begin;
      insert into t1 values (1);
      SET sql_mode = 'ANSI_QUOTES';
      savepoint `a``; create database couldbebadthingshere; savepoint ``dummy`;
      insert into t2 select * from t1;
      SET sql_mode = '';
      rollback to savepoint `a``; create database couldbebadthingshere; savepoint ``dummy`;
      insert into t1 values (3);
      commit;

      --source include/show_binlog_events.inc

      select * from t2;

      sync_slave_with_master;

      select * from t2;

      drop table t1, t2;

      --source include/rpl_end.inc

      The testcase will fail with

      Last_SQL_Error Error '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 'create database couldbebadthingshere; savepoint `dummy`' at line 1' on query. Default database: 'test'. Query: 'ROLLBACK TO `a`; create database couldbebadthingshere; savepoint `dummy`'

      and master binlog will have
      ...
      master-bin.000001 520 Query 1 657 SAVEPOINT "a`; create database couldbebadthingshere; savepoint `dummy"
      master-bin.000001 657 Query 1 762 use `test`; insert into t2 select * from t1
      master-bin.000001 762 Query 1 901 ROLLBACK TO `a`; create database couldbebadthingshere; savepoint `dummy`
      ...
      Note the lack of quoting in the ROLLBACK TO statement

      Suggested fix:
      append_identifier in binlog_savepoint_rollback

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              laurynas.biveinis Laurynas Biveinis (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Smart Checklist