PS-4561: Read after free at Binlog_crypt_data::load_latest_binlog_key()

Removed access to system_key after it was freed. Also initliazed nonce to 0s.
percona · Jul 26, 2018 · 77b404f · 77b404f
1 parent 43a77ae
commit 77b404f
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 8 deletions.
diff --git a/sql/binlog.cc b/sql/binlog.cc
@@ -5214,10 +5214,6 @@ bool MYSQL_BIN_LOG::open_binlog(const char *log_name,
 
   if (encrypt_binlog)
   {
-    uchar nonce[Binlog_crypt_data::BINLOG_NONCE_LENGTH];
-    if (my_rand_buffer(nonce, sizeof(nonce)))
-      goto err;
-
     if (crypto.load_latest_binlog_key())
     {
       sql_print_error("Failed to fetch or create percona_binlog key from/in keyring and thus "
@@ -5228,6 +5224,12 @@ bool MYSQL_BIN_LOG::open_binlog(const char *log_name,
     DBUG_EXECUTE_IF("check_consecutive_binlog_key_versions",
                     { static uint next_key_version = 0;
                       DBUG_ASSERT(crypto.get_key_version() == next_key_version++);});
+
+    uchar nonce[Binlog_crypt_data::BINLOG_NONCE_LENGTH];
+    memset(nonce, 0, Binlog_crypt_data::BINLOG_NONCE_LENGTH);
+    if (my_rand_buffer(nonce, sizeof(nonce)))
+      goto err;
+
     Start_encryption_log_event sele(1, crypto.get_key_version(), nonce);
     sele.common_footer->checksum_alg= s.common_footer->checksum_alg;
     if (write_to_file(&sele))

diff --git a/sql/binlog_crypt_data.cc b/sql/binlog_crypt_data.cc
@@ -81,6 +81,7 @@ Binlog_crypt_data& Binlog_crypt_data::operator=(Binlog_crypt_data b)
 bool Binlog_crypt_data::load_latest_binlog_key()
 {
   free_key(key, key_length);
+  bool error= false;
 #ifdef MYSQL_SERVER
   char *system_key_type = NULL;
   size_t system_key_len = 0;
@@ -98,13 +99,13 @@ bool Binlog_crypt_data::load_latest_binlog_key()
         system_key == NULL)))
          return true;
 
-  my_free(system_key_type);
   DBUG_ASSERT(strncmp(system_key_type, "AES", 3) == 0);
+  my_free(system_key_type);
 
-  if (parse_system_key(system_key, system_key_len, &key_version, &key, &key_length) == reinterpret_cast<uchar*>(NullS))
-    return true;
+  error= (parse_system_key(system_key, system_key_len, &key_version, &key, &key_length) == reinterpret_cast<uchar*>(NullS));
+  my_free(system_key);
 #endif
-  return false;
+  return error;
 }
 
 bool Binlog_crypt_data::init_with_loaded_key(uint sch, const uchar* nonce)