From cd2924bacbb06596d30f4397b0e200fc9c73e40b Mon Sep 17 00:00:00 2001
From: Sergei Golubchik <serg@mariadb.org>
Date: Fri, 7 Aug 2020 13:37:41 +0200
Subject: [PATCH] MDEV-23330 Server crash or ASAN negative-size-param in
 my_strnncollsp_binary / SORT_FIELD_ATTR::compare_packed_varstrings

and
MDEV-23414 Assertion `res->charset() == item->collation.collation' failed in Type_handler_string_result::make_packed_sort_key_part

pack_sort_string() *must* take a collation from the Item, not from the
String value. Because when casting a string to _binary the original
String is not copied for performance reasons, it's reused but its
collation does not match Item's collation anymore.

Note, that String's collation cannot be simply changed to _binary,
because for an Item_string literal the original String must stay
unchanged for the duration of the query.

this partially reverts 61c15ebe323
---
 mysql-test/main/func_crypt.result       |  1 +
 mysql-test/main/func_crypt.test         |  2 ++
 mysql-test/main/func_des_encrypt.result | 38 +++++++++++++++++++++++++
 mysql-test/main/func_des_encrypt.test   | 21 ++++++++++++++
 mysql-test/main/order_by.result         | 10 +++++++
 mysql-test/main/order_by.test           |  8 ++++++
 sql/field.cc                            |  2 +-
 sql/filesort.cc                         | 10 ++++---
 sql/item_timefunc.cc                    |  2 +-
 sql/sql_class.h                         |  3 +-
 sql/sql_string.h                        |  2 +-
 sql/unireg.cc                           |  4 +--
 12 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/mysql-test/main/func_crypt.result b/mysql-test/main/func_crypt.result
index aaa6aa61eae..2c42d3dc845 100644
--- a/mysql-test/main/func_crypt.result
+++ b/mysql-test/main/func_crypt.result
@@ -214,3 +214,4 @@ SELECT * FROM t1;
 a	b
 hello	12NKz5XM5JeKI
 DROP TABLE t1;
+# End of 10.2 tests
diff --git a/mysql-test/main/func_crypt.test b/mysql-test/main/func_crypt.test
index d091aa4ae86..118a7023669 100644
--- a/mysql-test/main/func_crypt.test
+++ b/mysql-test/main/func_crypt.test
@@ -120,3 +120,5 @@ SHOW CREATE TABLE t1;
 INSERT INTO t1 (a) VALUES ('hello');
 SELECT * FROM t1;
 DROP TABLE t1;
+
+--echo # End of 10.2 tests
diff --git a/mysql-test/main/func_des_encrypt.result b/mysql-test/main/func_des_encrypt.result
index b81f96f6ef7..540596589b6 100644
--- a/mysql-test/main/func_des_encrypt.result
+++ b/mysql-test/main/func_des_encrypt.result
@@ -35,3 +35,41 @@ DES_DECRYPT(DES_ENCRYPT('1234'))	DES_DECRYPT(DES_ENCRYPT('12345'))	DES_DECRYPT(D
 1234	12345	123456	1234567
 DROP TABLE t1;
 End of 5.0 tests
+#
+# MDEV-23330 Server crash or ASAN negative-size-param in
+# my_strnncollsp_binary / SORT_FIELD_ATTR::compare_packed_varstrings
+#
+CREATE TABLE t1 (a CHAR(240), b BIT(48));
+INSERT INTO t1 VALUES ('a',b'0001'),('b',b'0010'),('c',b'0011'),('d',b'0100'),('e',b'0001'),('f',b'0101'),('g',b'0110'),('h',b'0111'),('i',b'1000'),('j',b'1001');
+SELECT DES_DECRYPT(a, 'x'), HEX(BINARY b) FROM t1 GROUP BY 1, 2 WITH ROLLUP;
+DES_DECRYPT(a, 'x')	HEX(BINARY b)
+a	000000000001
+a	NULL
+b	000000000002
+b	NULL
+c	000000000003
+c	NULL
+d	000000000004
+d	NULL
+e	000000000001
+e	NULL
+f	000000000005
+f	NULL
+g	000000000006
+g	NULL
+h	000000000007
+h	NULL
+i	000000000008
+i	NULL
+j	000000000009
+j	NULL
+NULL	NULL
+DROP TABLE t1;
+CREATE TABLE t1 (a INT);
+INSERT t1 VALUES (1),(2);
+SELECT CHAR_LENGTH(a), DES_DECRYPT(a) FROM (SELECT _utf8 0xC2A2 AS a FROM t1) AS t2;
+CHAR_LENGTH(a)	DES_DECRYPT(a)
+1	¢
+1	¢
+DROP TABLE t1;
+End of 10.5 tests
diff --git a/mysql-test/main/func_des_encrypt.test b/mysql-test/main/func_des_encrypt.test
index c9661b81cc0..44fc30ff00f 100644
--- a/mysql-test/main/func_des_encrypt.test
+++ b/mysql-test/main/func_des_encrypt.test
@@ -37,3 +37,24 @@ SELECT
 DROP TABLE t1;
 
 --Echo End of 5.0 tests
+
+--echo #
+--echo # MDEV-23330 Server crash or ASAN negative-size-param in
+--echo # my_strnncollsp_binary / SORT_FIELD_ATTR::compare_packed_varstrings
+--echo #
+
+CREATE TABLE t1 (a CHAR(240), b BIT(48));
+INSERT INTO t1 VALUES ('a',b'0001'),('b',b'0010'),('c',b'0011'),('d',b'0100'),('e',b'0001'),('f',b'0101'),('g',b'0110'),('h',b'0111'),('i',b'1000'),('j',b'1001');
+SELECT DES_DECRYPT(a, 'x'), HEX(BINARY b) FROM t1 GROUP BY 1, 2 WITH ROLLUP;
+DROP TABLE t1;
+
+#
+# don't change the charset of a literal Item_string
+#
+
+CREATE TABLE t1 (a INT);
+INSERT t1 VALUES (1),(2);
+SELECT CHAR_LENGTH(a), DES_DECRYPT(a) FROM (SELECT _utf8 0xC2A2 AS a FROM t1) AS t2;
+DROP TABLE t1;
+
+--Echo End of 10.5 tests
diff --git a/mysql-test/main/order_by.result b/mysql-test/main/order_by.result
index f1a6cb086b8..de0d3dc4003 100644
--- a/mysql-test/main/order_by.result
+++ b/mysql-test/main/order_by.result
@@ -4097,4 +4097,14 @@ Y
 B
 A
 DROP TABLE t1;
+#
+# MDEV-23414 Assertion `res->charset() == item->collation.collation' failed in Type_handler_string_result::make_packed_sort_key_part
+#
+CREATE TABLE t1 (a CHAR(3), b BINARY(255));
+INSERT t1 VALUES ('foo','bar'),('baz','qux');
+SELECT COALESCE(a, b) AS f FROM t1 ORDER BY f;
+f
+baz
+foo
+DROP TABLE t1;
 # End of 10.5 tests
diff --git a/mysql-test/main/order_by.test b/mysql-test/main/order_by.test
index e27822006b5..038bf82b0fb 100644
--- a/mysql-test/main/order_by.test
+++ b/mysql-test/main/order_by.test
@@ -2533,4 +2533,12 @@ SELECT * FROM t1 ORDER BY a DESC;
 
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-23414 Assertion `res->charset() == item->collation.collation' failed in Type_handler_string_result::make_packed_sort_key_part
+--echo #
+CREATE TABLE t1 (a CHAR(3), b BINARY(255));
+INSERT t1 VALUES ('foo','bar'),('baz','qux');
+SELECT COALESCE(a, b) AS f FROM t1 ORDER BY f;
+DROP TABLE t1;
+
 --echo # End of 10.5 tests
diff --git a/sql/field.cc b/sql/field.cc
index 0f4837f20f0..f50ddec1c80 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -1087,7 +1087,7 @@ Field_longstr::pack_sort_string(uchar *to, const SORT_FIELD_ATTR *sort_field)
 {
   StringBuffer<LONGLONG_BUFFER_SIZE> buf;
   val_str(&buf, &buf);
-  return to + sort_field->pack_sort_string(to, &buf);
+  return to + sort_field->pack_sort_string(to, &buf, field_charset());
 }
 
 
diff --git a/sql/filesort.cc b/sql/filesort.cc
index ec82cea9775..90fbde6f6c8 100644
--- a/sql/filesort.cc
+++ b/sql/filesort.cc
@@ -2544,12 +2544,13 @@ Type_handler_string_result::make_packed_sort_key_part(uchar *to, Item *item,
                                             const SORT_FIELD_ATTR *sort_field,
                                             Sort_param *param) const
 {
+  CHARSET_INFO *cs= item->collation.collation;
   bool maybe_null= item->maybe_null;
 
   if (maybe_null)
     *to++= 1;
 
-  String *res= item->str_result(&param->tmp_buffer);
+  Binary_string *res= item->str_result(&param->tmp_buffer);
   if (!res)
   {
     if (maybe_null)
@@ -2573,7 +2574,7 @@ Type_handler_string_result::make_packed_sort_key_part(uchar *to, Item *item,
       return sort_field->original_length;
     }
   }
-  return sort_field->pack_sort_string(to, res);
+  return sort_field->pack_sort_string(to, res, cs);
 }
 
 
@@ -2937,7 +2938,8 @@ int compare_packed_sort_keys(void *sort_param,
 */
 
 uint
-SORT_FIELD_ATTR::pack_sort_string(uchar *to, String *str) const
+SORT_FIELD_ATTR::pack_sort_string(uchar *to, const Binary_string *str,
+                                  CHARSET_INFO *cs) const
 {
   uchar *orig_to= to;
   uint32 length, data_length;
@@ -2956,7 +2958,7 @@ SORT_FIELD_ATTR::pack_sort_string(uchar *to, String *str) const
   memcpy(to, (uchar*)str->ptr(), data_length);
   to+= data_length;
 
-  if (str->charset() == &my_charset_bin && suffix_length)
+  if (cs == &my_charset_bin && suffix_length)
   {
     // suffix length stored in bigendian form
     store_bigendian(length, to, suffix_length);
diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc
index 052b11f8b04..6b5c6fa323c 100644
--- a/sql/item_timefunc.cc
+++ b/sql/item_timefunc.cc
@@ -474,7 +474,7 @@ static bool extract_date_time(THD *thd, DATE_TIME_FORMAT *format,
   Create a formatted date/time value in a string.
 */
 
-static bool make_date_time(String *format, MYSQL_TIME *l_time,
+static bool make_date_time(const String *format, const MYSQL_TIME *l_time,
                            timestamp_type type, const MY_LOCALE *locale,
                            String *str)
 {
diff --git a/sql/sql_class.h b/sql/sql_class.h
index b56de95cdd6..7e512048bce 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -6454,7 +6454,8 @@ struct SORT_FIELD_ATTR
   */
   bool maybe_null;
   CHARSET_INFO *cs;
-  uint pack_sort_string(uchar *to, String *str) const;
+  uint pack_sort_string(uchar *to, const Binary_string *str,
+                        CHARSET_INFO *cs) const;
   int compare_packed_fixed_size_vals(uchar *a, size_t *a_len,
                                      uchar *b, size_t *b_len);
   int compare_packed_varstrings(uchar *a, size_t *a_len,
diff --git a/sql/sql_string.h b/sql/sql_string.h
index 274b1d9a5df..37e9c40db61 100644
--- a/sql/sql_string.h
+++ b/sql/sql_string.h
@@ -882,7 +882,7 @@ class String: public Charset, public Binary_string
   {
     return Binary_string::append_hex((const char*)src, srclen);
   }
-  bool append_introducer_and_hex(String *str)
+  bool append_introducer_and_hex(const String *str)
   {
     return
       append(STRING_WITH_LEN("_"))   ||
diff --git a/sql/unireg.cc b/sql/unireg.cc
index ae860f0143b..c299549843f 100644
--- a/sql/unireg.cc
+++ b/sql/unireg.cc
@@ -86,7 +86,7 @@ static uchar* extra2_write_str(uchar *pos, const LEX_CSTRING &str)
   return pos + str.length;
 }
 
-static uchar* extra2_write_str(uchar *pos, Binary_string *str)
+static uchar* extra2_write_str(uchar *pos, const Binary_string *str)
 {
   pos= extra2_write_len(pos, str->length());
   memcpy(pos, str->ptr(), str->length());
@@ -185,7 +185,7 @@ class Field_data_type_info_image: public BinaryStringBuffer<512>
   {
     return net_store_length(pos, length);
   }
-  static uchar *store_string(uchar *pos, Binary_string *str)
+  static uchar *store_string(uchar *pos, const Binary_string *str)
   {
     pos= store_length(pos, str->length());
     memcpy(pos, str->ptr(), str->length());
-- 
2.30.9