[Commits] 0187bb03884: MDEV_-17589: Stack-buffer-overflow with indexed varchar (utf8) field
revision-id: 0187bb03884a34ca8de96e47f4cb1f3d860e6db9 (mariadb-10.2.16-224-g0187bb03884) parent(s): 8a346f31b913daa011085afec2b2d38450c73e00 author: Varun Gupta committer: Varun Gupta timestamp: 2018-11-05 19:32:26 +0530 message: MDEV_-17589: Stack-buffer-overflow with indexed varchar (utf8) field Create a new constant MAX_DATA_LENGTH_FOR_KEY. Replace the value of MAX_KEY_LENGTH to also include the LENGTH and NULL BYTES. --- mysql-test/r/func_group_innodb.result | 23 +++++++++++++++++++++++ mysql-test/r/innodb_ext_key.result | 4 ++-- mysql-test/r/partition_datatype.result | 2 +- mysql-test/r/partition_utf8.result | 2 +- mysql-test/t/func_group_innodb.test | 18 ++++++++++++++++++ mysql-test/t/partition_datatype.test | 2 +- mysql-test/t/partition_utf8.test | 2 +- sql/handler.h | 10 +++++++--- sql/sql_const.h | 2 +- 9 files changed, 55 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/func_group_innodb.result b/mysql-test/r/func_group_innodb.result index 52d5922df95..c2bfe9bf81c 100644 --- a/mysql-test/r/func_group_innodb.result +++ b/mysql-test/r/func_group_innodb.result @@ -246,4 +246,27 @@ EXPLAIN SELECT MIN(c) FROM t1 GROUP BY b; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t1 range NULL b 263 NULL 3 Using index for group-by DROP TABLE t1; +# +# MDEV-17589: Stack-buffer-overflow with indexed varchar (utf8) field +# +CREATE TABLE t1 (v1 varchar(1020), v2 varchar(2), v3 varchar(2), KEY k1 (v3,v2,v1)) ENGINE=InnoDB CHARACTER SET=utf8; +INSERT INTO t1 VALUES ('king', 'qu','qu'), ('bad','go','go'); +explain +SELECT MIN(t1.v1) FROM t1 where t1.v2='qu' and t1.v3='qu'; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Select tables optimized away +SELECT MIN(t1.v1) FROM t1 where t1.v2='qu' and t1.v3='qu'; +MIN(t1.v1) +king +drop table t1; +CREATE TABLE t1 (v1 varchar(1024) CHARACTER SET utf8, KEY v1 (v1)) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('king'), ('bad'); +explain +SELECT MIN(x.v1) FROM (SELECT t1.* FROM t1 WHERE t1.v1 >= 'p') x; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE NULL NULL NULL NULL NULL NULL NULL No matching min/max row +SELECT MIN(x.v1) FROM (SELECT t1.* FROM t1 WHERE t1.v1 >= 'p') x; +MIN(x.v1) +NULL +drop table t1; End of 5.5 tests diff --git a/mysql-test/r/innodb_ext_key.result b/mysql-test/r/innodb_ext_key.result index c55e8d138f8..c76c8613c57 100644 --- a/mysql-test/r/innodb_ext_key.result +++ b/mysql-test/r/innodb_ext_key.result @@ -1168,8 +1168,8 @@ EXPLAIN "access_type": "range", "possible_keys": ["f2"], "key": "f2", - "key_length": "3070", - "used_key_parts": ["f2", "pk1"], + "key_length": "3074", + "used_key_parts": ["f2", "pk1", "pk2"], "rows": 1, "filtered": 100, "index_condition": "t1.pk1 <= 5 and t1.pk2 <= 5 and t1.f2 = 'abc'", diff --git a/mysql-test/r/partition_datatype.result b/mysql-test/r/partition_datatype.result index 2e518c194f0..66d003c97cb 100644 --- a/mysql-test/r/partition_datatype.result +++ b/mysql-test/r/partition_datatype.result @@ -314,7 +314,7 @@ bbbb drop table t1; set sql_mode=''; create table t1 (a varchar(3070)) partition by key (a); -ERROR HY000: The total length of the partitioning fields is too large +drop table t1; create table t1 (a varchar(65532) not null) partition by key (a); ERROR HY000: The total length of the partitioning fields is too large create table t1 (a varchar(65533)) partition by key (a); diff --git a/mysql-test/r/partition_utf8.result b/mysql-test/r/partition_utf8.result index 7718e651423..5c0d4d86e8e 100644 --- a/mysql-test/r/partition_utf8.result +++ b/mysql-test/r/partition_utf8.result @@ -24,7 +24,7 @@ drop table t1; create table t1 (a varchar(1500), b varchar(1570)) partition by list columns(a,b) ( partition p0 values in (('a','b'))); -ERROR HY000: The total length of the partitioning fields is too large +drop table t1; create table t1 (a varchar(1023) character set utf8 collate utf8_spanish2_ci) partition by range columns(a) ( partition p0 values less than ('CZ'), diff --git a/mysql-test/t/func_group_innodb.test b/mysql-test/t/func_group_innodb.test index c62d3d08496..0d24f37363b 100644 --- a/mysql-test/t/func_group_innodb.test +++ b/mysql-test/t/func_group_innodb.test @@ -192,4 +192,22 @@ EXPLAIN SELECT MIN(c) FROM t1 GROUP BY b; DROP TABLE t1; +--echo # +--echo # MDEV-17589: Stack-buffer-overflow with indexed varchar (utf8) field +--echo # + +CREATE TABLE t1 (v1 varchar(1020), v2 varchar(2), v3 varchar(2), KEY k1 (v3,v2,v1)) ENGINE=InnoDB CHARACTER SET=utf8; +INSERT INTO t1 VALUES ('king', 'qu','qu'), ('bad','go','go'); +explain +SELECT MIN(t1.v1) FROM t1 where t1.v2='qu' and t1.v3='qu'; +SELECT MIN(t1.v1) FROM t1 where t1.v2='qu' and t1.v3='qu'; +drop table t1; + +CREATE TABLE t1 (v1 varchar(1024) CHARACTER SET utf8, KEY v1 (v1)) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('king'), ('bad'); +explain +SELECT MIN(x.v1) FROM (SELECT t1.* FROM t1 WHERE t1.v1 >= 'p') x; +SELECT MIN(x.v1) FROM (SELECT t1.* FROM t1 WHERE t1.v1 >= 'p') x; +drop table t1; + --echo End of 5.5 tests diff --git a/mysql-test/t/partition_datatype.test b/mysql-test/t/partition_datatype.test index 9ab3bd4d5fa..96f7da96e3d 100644 --- a/mysql-test/t/partition_datatype.test +++ b/mysql-test/t/partition_datatype.test @@ -216,8 +216,8 @@ select * from t1 where a like 'aaa%'; select * from t1 where a = 'bbbb'; drop table t1; set sql_mode=''; --- error ER_PARTITION_FIELDS_TOO_LONG create table t1 (a varchar(3070)) partition by key (a); +drop table t1; -- error ER_PARTITION_FIELDS_TOO_LONG create table t1 (a varchar(65532) not null) partition by key (a); -- error ER_BLOB_FIELD_IN_PART_FUNC_ERROR diff --git a/mysql-test/t/partition_utf8.test b/mysql-test/t/partition_utf8.test index d3ad7ba671e..b806abe0a98 100644 --- a/mysql-test/t/partition_utf8.test +++ b/mysql-test/t/partition_utf8.test @@ -15,10 +15,10 @@ drop table t1; # # BUG#48164, too long partition fields causes crash # ---error ER_PARTITION_FIELDS_TOO_LONG create table t1 (a varchar(1500), b varchar(1570)) partition by list columns(a,b) ( partition p0 values in (('a','b'))); +drop table t1; create table t1 (a varchar(1023) character set utf8 collate utf8_spanish2_ci) partition by range columns(a) diff --git a/sql/handler.h b/sql/handler.h index ed2ef822c88..75f72bdfb53 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -378,6 +378,10 @@ enum enum_alter_inplace_result { #define HA_KEY_NULL_LENGTH 1 #define HA_KEY_BLOB_LENGTH 2 +#define MAX_KEY_LENGTH (MAX_DATA_LENGTH_FOR_KEY \ + +(MAX_REF_PARTS \ + *(HA_KEY_NULL_LENGTH + HA_KEY_BLOB_LENGTH))) + #define HA_LEX_CREATE_TMP_TABLE 1U #define HA_CREATE_TMP_ALTER 8U @@ -3421,14 +3425,14 @@ class handler :public Sql_alloc uint max_key_parts() const { return MY_MIN(MAX_REF_PARTS, max_supported_key_parts()); } uint max_key_length() const - { return MY_MIN(MAX_KEY_LENGTH, max_supported_key_length()); } + { return MY_MIN(MAX_DATA_LENGTH_FOR_KEY, max_supported_key_length()); } uint max_key_part_length() const - { return MY_MIN(MAX_KEY_LENGTH, max_supported_key_part_length()); } + { return MY_MIN(MAX_DATA_LENGTH_FOR_KEY, max_supported_key_part_length()); } virtual uint max_supported_record_length() const { return HA_MAX_REC_LENGTH; } virtual uint max_supported_keys() const { return 0; } virtual uint max_supported_key_parts() const { return MAX_REF_PARTS; } - virtual uint max_supported_key_length() const { return MAX_KEY_LENGTH; } + virtual uint max_supported_key_length() const { return MAX_DATA_LENGTH_FOR_KEY; } virtual uint max_supported_key_part_length() const { return 255; } virtual uint min_record_length(uint options) const { return 1; } diff --git a/sql/sql_const.h b/sql/sql_const.h index 7395ae3c08a..0be63a2a5ad 100644 --- a/sql/sql_const.h +++ b/sql/sql_const.h @@ -33,7 +33,7 @@ #define MAX_SYS_VAR_LENGTH 32 #define MAX_KEY MAX_INDEXES /* Max used keys */ #define MAX_REF_PARTS 32 /* Max parts used as ref */ -#define MAX_KEY_LENGTH 3072 /* max possible key */ +#define MAX_DATA_LENGTH_FOR_KEY 3072 #if SIZEOF_OFF_T > 4 #define MAX_REFLENGTH 8 /* Max length for record ref */ #else
Hi Varun, On Mon, Nov 05, 2018 at 07:36:17PM +0530, Varun wrote:
revision-id: 0187bb03884a34ca8de96e47f4cb1f3d860e6db9 (mariadb-10.2.16-224-g0187bb03884) parent(s): 8a346f31b913daa011085afec2b2d38450c73e00 author: Varun Gupta committer: Varun Gupta timestamp: 2018-11-05 19:32:26 +0530 message:
MDEV_-17589: Stack-buffer-overflow with indexed varchar (utf8) field
Create a new constant MAX_DATA_LENGTH_FOR_KEY. Replace the value of MAX_KEY_LENGTH to also include the LENGTH and NULL BYTES.
...
diff --git a/mysql-test/r/partition_datatype.result b/mysql-test/r/partition_datatype.result index 2e518c194f0..66d003c97cb 100644 --- a/mysql-test/r/partition_datatype.result +++ b/mysql-test/r/partition_datatype.result @@ -314,7 +314,7 @@ bbbb drop table t1; set sql_mode=''; create table t1 (a varchar(3070)) partition by key (a); -ERROR HY000: The total length of the partitioning fields is too large +drop table t1;
I don't think it's a good idea that after this patch we allow the table definitions that we didn't allow before. This raises the question of datadir downgrades, etc. I think we should only remove the buffer overrun. Here's patch how to avoid the above: https://gist.github.com/spetrunia/7e7e3fcc60a81a6cb854e77c12c367d3
create table t1 (a varchar(65532) not null) partition by key (a); ERROR HY000: The total length of the partitioning fields is too large create table t1 (a varchar(65533)) partition by key (a); ...
diff --git a/mysql-test/r/innodb_ext_key.result b/mysql-test/r/innodb_ext_key.result index c55e8d138f8..c76c8613c57 100644 --- a/mysql-test/r/innodb_ext_key.result +++ b/mysql-test/r/innodb_ext_key.result @@ -1168,8 +1168,8 @@ EXPLAIN "access_type": "range", "possible_keys": ["f2"], "key": "f2", - "key_length": "3070", - "used_key_parts": ["f2", "pk1"], + "key_length": "3074", + "used_key_parts": ["f2", "pk1", "pk2"],
Same reaction as above: This change shows that "Extended keys" feature now allows bigger set of key extensions than before. Can we avoid making this change by changing MAX_KEY_LENGTH to MAX_DATA_LENGTH_FOR_KEY somewhere?
"rows": 1, "filtered": 100, "index_condition": "t1.pk1 <= 5 and t1.pk2 <= 5 and t1.f2 = 'abc'",
diff --git a/sql/handler.h b/sql/handler.h index ed2ef822c88..75f72bdfb53 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -378,6 +378,10 @@ enum enum_alter_inplace_result { #define HA_KEY_NULL_LENGTH 1 #define HA_KEY_BLOB_LENGTH 2
+#define MAX_KEY_LENGTH (MAX_DATA_LENGTH_FOR_KEY \ + +(MAX_REF_PARTS \ + *(HA_KEY_NULL_LENGTH + HA_KEY_BLOB_LENGTH))) + I think this requires a comment. Please add something like
/* Maximum length of any index lookup key, in bytes */
#define HA_LEX_CREATE_TMP_TABLE 1U #define HA_CREATE_TMP_ALTER 8U
@@ -3421,14 +3425,14 @@ class handler :public Sql_alloc uint max_key_parts() const { return MY_MIN(MAX_REF_PARTS, max_supported_key_parts()); } uint max_key_length() const - { return MY_MIN(MAX_KEY_LENGTH, max_supported_key_length()); } + { return MY_MIN(MAX_DATA_LENGTH_FOR_KEY, max_supported_key_length()); } uint max_key_part_length() const - { return MY_MIN(MAX_KEY_LENGTH, max_supported_key_part_length()); } + { return MY_MIN(MAX_DATA_LENGTH_FOR_KEY, max_supported_key_part_length()); }
virtual uint max_supported_record_length() const { return HA_MAX_REC_LENGTH; } virtual uint max_supported_keys() const { return 0; } virtual uint max_supported_key_parts() const { return MAX_REF_PARTS; } - virtual uint max_supported_key_length() const { return MAX_KEY_LENGTH; } + virtual uint max_supported_key_length() const { return MAX_DATA_LENGTH_FOR_KEY; } virtual uint max_supported_key_part_length() const { return 255; } virtual uint min_record_length(uint options) const { return 1; }
diff --git a/sql/sql_const.h b/sql/sql_const.h index 7395ae3c08a..0be63a2a5ad 100644 --- a/sql/sql_const.h +++ b/sql/sql_const.h @@ -33,7 +33,7 @@ #define MAX_SYS_VAR_LENGTH 32 #define MAX_KEY MAX_INDEXES /* Max used keys */ #define MAX_REF_PARTS 32 /* Max parts used as ref */
Please add a comment, something like /* Maximum length of the data part of an index lookup key. The "data part" is defined as the value itself, not including the NULL-indicator bytes or varchar length bytes ("the Extras"). We need this value because there was a bug where length of the Extras were not counted. You probably need MAX_KEY_LENGTH, not this constant. */
-#define MAX_KEY_LENGTH 3072 /* max possible key */ +#define MAX_DATA_LENGTH_FOR_KEY 3072 #if SIZEOF_OFF_T > 4 #define MAX_REFLENGTH 8 /* Max length for record ref */ #else
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
participants (2)
-
Sergey Petrunia
-
varunraiko1803@gmail.com