Bug#30885987 - ANOTHER BUFFER OVERFLOWS IN CLI_READ_ROWS

Anushree Prakash B · Anushree Prakash B · commit 3099bab30045 · 2020-11-18T20:14:55.000+05:30
DESCRIPTION
===========
There is a heap buffer over read and over write while
reading rows if the packet contents are bad.

ANALYSIS
========
There are some missing boundary checks in client-side
functions which can cause read or write buffer overflows.

FIX
===
Added checks in the required code path to avoid buffer
overflows.

RB: 25367
diff --git a/mysql-test/include/mysql_have_debug.inc b/mysql-test/include/mysql_have_debug.inc
@@ -0,0 +1,33 @@
+#######################################################
+# checks if mysql is debug compiled.
+# This "cannot" be done simply by using have_debug.inc
+######################################################
+
+--disable_query_log
+--let $temp_out_help_file=$MYSQL_TMP_DIR/mysql_help.tmp
+--exec $MYSQL --help>$temp_out_help_file
+let log_tmp=$temp_out_help_file;
+--let $temp_inc=$MYSQL_TMP_DIR/temp.inc
+let inc_tmp=$temp_inc;
+
+--perl
+use strict;
+my $tmp_file= $ENV{'log_tmp'} or die "log_tmp not set";
+open(FILE, "$tmp_file") or die("Unable to open $tmp_file: $!\n");
+my $count = () = grep(/Output debug log/g,<FILE>);
+close FILE;
+
+my $temp_inc= $ENV{'inc_tmp'} or die "temp_inc not set";
+open(FILE_INC,">", "$temp_inc") or die("can't open file \"$temp_inc\": $!");
+print FILE_INC '--let $is_debug= '.$count;
+close FILE_INC;
+EOF
+--source $temp_inc
+
+if (!$is_debug)
+{
+  --skip mysql needs to be debug compiled
+}
+--remove_file $temp_out_help_file
+--remove_file $temp_inc
+--enable_query_log
diff --git a/sql-common/client.c b/sql-common/client.c
@@ -1695,7 +1695,20 @@ read_one_row(MYSQL *mysql,uint fields,MYSQL_ROW row, ulong *lengths)
   end_pos=pos+pkt_len;
   for (field=0 ; field < fields ; field++)
   {
+    if (pos >= end_pos) {
+      set_mysql_error(mysql, CR_MALFORMED_PACKET, unknown_sqlstate);
+      return -1;
+    }
     len=(ulong) net_field_length_checked(&pos, (ulong)(end_pos - pos));
+    DBUG_EXECUTE_IF("simulate_bad_field_length_1", {
+      len= 1000000L;
+      fields= 2;
+    });
+    DBUG_EXECUTE_IF("simulate_bad_field_length_2", {
+      len= pkt_len - 1;
+      fields= 2;
+    });
+
     if (pos > end_pos)
     {
       set_mysql_error(mysql, CR_UNKNOWN_ERROR, unknown_sqlstate);
@@ -1713,12 +1726,18 @@ read_one_row(MYSQL *mysql,uint fields,MYSQL_ROW row, ulong *lengths)
       pos+=len;
       *lengths++=len;
     }
+
+    /*
+     It's safe to write to prev_pos here because we already check
+     for a valid pos in the beginning of this loop.
+    */
     if (prev_pos)
       *prev_pos=0;				/* Terminate prev field */
     prev_pos=pos;
   }
   row[field]=(char*) prev_pos+1;		/* End of last field */
-  *prev_pos=0;					/* Terminate last field */
+  if (prev_pos < end_pos)
+    *prev_pos=0;					/* Terminate last field */
   return 0;
 }
 
diff --git a/sql/net_serv.cc b/sql/net_serv.cc
@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -215,6 +215,17 @@ void net_clear(NET *net,
 {
   DBUG_ENTER("net_clear");
 
+  DBUG_EXECUTE_IF("simulate_bad_field_length_1", {
+    net->pkt_nr= net->compress_pkt_nr= 0;
+    net->write_pos= net->buff;
+    DBUG_VOID_RETURN;
+  });
+  DBUG_EXECUTE_IF("simulate_bad_field_length_2", {
+    net->pkt_nr= net->compress_pkt_nr= 0;
+    net->write_pos= net->buff;
+    DBUG_VOID_RETURN;
+  });
+
 #if !defined(EMBEDDED_LIBRARY)
   /* Ensure the socket buffer is empty, except for an EOF (at least 1). */
   DBUG_ASSERT(!check_buffer || (vio_pending(net->vio) <= 1));
