diff options
| -rw-r--r-- | sql/sql_prepare.cc | 148 | ||||
| -rw-r--r-- | tests/mysql_client_test.c | 148 |
2 files changed, 288 insertions, 8 deletions
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index b3c5700a0b7..33695e4599b 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -123,6 +123,9 @@ When one supplies long data for a placeholder: #include "transaction.h" // trans_rollback_implicit #include "wsrep_mysqld.h" +/* Constants defining bits in parameter type flags. Flags are read from high byte of short value */ +static const uint parameter_flag_unsigned = 128U << 8; + /** A result class used to send cursor rows using the binary protocol. */ @@ -949,11 +952,67 @@ static bool insert_bulk_params(Prepared_statement *stmt, DBUG_RETURN(0); } -static bool set_conversion_functions(Prepared_statement *stmt, - uchar **data, uchar *data_end) +/** + Checking if parameter type and flags are valid + @param typecode - ushort value with type in low byte, and flags in high byte +*/ +bool +parameter_type_sanity_check(ushort typecode) +{ + /* Checking if type in lower byte is valid */ + switch (typecode & 0xff) { + case MYSQL_TYPE_DECIMAL: + case MYSQL_TYPE_NEWDECIMAL: + case MYSQL_TYPE_TINY: + case MYSQL_TYPE_SHORT: + case MYSQL_TYPE_LONG: + case MYSQL_TYPE_LONGLONG: + case MYSQL_TYPE_INT24: + case MYSQL_TYPE_YEAR: + case MYSQL_TYPE_BIT: + case MYSQL_TYPE_FLOAT: + case MYSQL_TYPE_DOUBLE: + case MYSQL_TYPE_NULL: + case MYSQL_TYPE_VARCHAR: + case MYSQL_TYPE_TINY_BLOB: + case MYSQL_TYPE_MEDIUM_BLOB: + case MYSQL_TYPE_LONG_BLOB: + case MYSQL_TYPE_BLOB: + case MYSQL_TYPE_VAR_STRING: + case MYSQL_TYPE_STRING: + case MYSQL_TYPE_ENUM: + case MYSQL_TYPE_SET: + case MYSQL_TYPE_GEOMETRY: + case MYSQL_TYPE_TIMESTAMP: + case MYSQL_TYPE_DATE: + case MYSQL_TYPE_TIME: + case MYSQL_TYPE_DATETIME: + // This types normally cannot be sent by client, so maybe it'd be better to treat them like an error here + case MYSQL_TYPE_TIMESTAMP2: + case MYSQL_TYPE_TIME2: + case MYSQL_TYPE_DATETIME2: + case MYSQL_TYPE_NEWDATE: + case MYSQL_TYPE_VARCHAR_COMPRESSED: + case MYSQL_TYPE_BLOB_COMPRESSED: + break; + default: + return true; + }; + + // In Flags in high byte only unsigned bit may be set + if (typecode & ((~parameter_flag_unsigned) & 0x0000ff00)) + { + return true; + } + return false; +} + +static bool +set_conversion_functions(Prepared_statement *stmt, + uchar **data, uchar *data_end) { uchar *read_pos= *data; - const uint signed_bit= 1 << 15; + DBUG_ENTER("set_conversion_functions"); /* First execute or types altered by the client, setup the @@ -966,12 +1025,15 @@ static bool set_conversion_functions(Prepared_statement *stmt, { ushort typecode; - if (read_pos >= data_end) - DBUG_RETURN(1); - + /* stmt_execute_packet_sanity_check has already verified, that there are enough data in the packet for data types */ typecode= sint2korr(read_pos); read_pos+= 2; - (**it).unsigned_flag= MY_TEST(typecode & signed_bit); + + if (parameter_type_sanity_check(typecode)) + { + DBUG_RETURN(1); + } + (**it).unsigned_flag= MY_TEST(typecode & parameter_flag_unsigned); (*it)->setup_conversion(thd, (uchar) (typecode & 0xff)); (*it)->sync_clones(); } @@ -3126,11 +3188,20 @@ static void mysql_stmt_execute_common(THD *thd, void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length) { + const uint packet_min_lenght= 9; uchar *packet= (uchar*)packet_arg; // GCC 4.0.1 workaround + + DBUG_ENTER("mysqld_stmt_execute"); + + if (packet_length < packet_min_lenght) + { + my_error(ER_MALFORMED_PACKET, MYF(0), 0, + "", "mysqld_stmt_execute"); + DBUG_VOID_RETURN; + } ulong stmt_id= uint4korr(packet); ulong flags= (ulong) packet[4]; uchar *packet_end= packet + packet_length; - DBUG_ENTER("mysqld_stmt_execute"); packet+= 9; /* stmt_id + 5 bytes of flags */ @@ -3186,7 +3257,54 @@ void mysqld_stmt_bulk_execute(THD *thd, char *packet_arg, uint packet_length) DBUG_VOID_RETURN; } +/** +Additional packet checks for direct execution + +@param thd THD handle +@param stmt prepared statement being directly executed +@param paket packet with parameters to bind +@param packet_end pointer to the byte after parameters end +@param bulk_op is it bulk operation +@param direct_exec is it direct execution +*/ +bool stmt_execute_packet_sanity_check(Prepared_statement *stmt, uchar *packet, uchar *packet_end, bool bulk_op, bool direct_exec) +{ + if (stmt->param_count > 0) + { + uint packet_length= packet_end - packet; + uint null_bitmap_bytes= (uint)(bulk_op ? 0 : (stmt->param_count + 7)/8); + uint min_len_for_param_count = null_bitmap_bytes + + 1; /* sent types byte */ + + if (packet_length >= min_len_for_param_count) + { + if (!bulk_op && packet[null_bitmap_bytes]) + { + /* Should be 0 or 1. If the byte is not 1, that could mean, + e.g. that we read incorrect byte due to incorrect number of sent parameters for direct execution + (i.e. null bitmap is shorter or longer, than it should be) */ + if (packet[null_bitmap_bytes] != '\1') + { + return true; + } + min_len_for_param_count+= 2*stmt->param_count; /* 2 bytes per parameter of the type and flags */ + } + else /* If types are not sent, there is nothing to do here. But for direct execution types should always be sent */ + { + return direct_exec; + } + } + /* If true, the packet is guaranteed too short for the number of parameters in the PS */ + return (packet_length < min_len_for_param_count); + } + else + { + /* If there is no parameters, this should be normally already end of the packet. If it's not - then error */ + return (packet_end > packet); + } + return false; +} /** Common part of prepared statement execution @@ -3225,6 +3343,20 @@ static void mysql_stmt_execute_common(THD *thd, llstr(stmt_id, llbuf), "mysqld_stmt_execute"); DBUG_VOID_RETURN; } + + /* + * In case of direct execution application decides how many parameters to send. + * Thus extra checks are reuqired to prevent crashes caused by incorrect interpretation of + * the packet data. Plus there can be always a broken evil client. + */ + if (stmt_execute_packet_sanity_check(stmt, packet, packet_end, bulk_op, stmt_id == LAST_STMT_ID)) + { + char llbuf[22]; + my_error(ER_MALFORMED_PACKET, MYF(0), static_cast<int>(sizeof(llbuf)), + llstr(stmt_id, llbuf), "mysqld_stmt_execute"); + DBUG_VOID_RETURN; + } + stmt->read_types= read_types; #if defined(ENABLED_PROFILING) diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index edad6453102..0ed18a05bed 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -36,6 +36,9 @@ #endif #include "mysql_client_fw.c" +#ifndef MARIADB_STMT_BULK_SUPPORTED +#include "mariadb_stmt.h" +#endif // !MARIADB_STMT_BULK_SUPPORTED #ifndef _WIN32 #include <arpa/inet.h> #endif @@ -20860,6 +20863,150 @@ static void test_explain_meta() } +#define MDEV19838_MAX_PARAM_COUNT 32 +#define MDEV19838_FIELDS_COUNT 17 +static void test_mdev19838() +{ + int rc; + MYSQL_BIND bind[MDEV19838_MAX_PARAM_COUNT]; + unsigned int i, paramCount = 1; + char charvalue[] = "012345678901234567890123456789012345"; + MYSQL_STMT *stmt; + + myheader("test_mdev19838"); + + rc = mysql_query(mysql, "CREATE TABLE mdev19838(" + "f1 char(36)," + "f2 char(36)," + "f3 char(36)," + "f4 char(36)," + "f5 char(36)," + "f6 char(36)," + "f7 char(36)," + "f8 char(36)," + "f9 char(36)," + "f10 char(36)," + "f11 char(36)," + "f12 char(36)," + "f13 char(36)," + "f14 char(36)," + "f15 char(36)," + "f16 char(36)," + "f17 char(36)" + ")"); + myquery(rc); + + stmt = mysql_stmt_init(mysql); + check_stmt(stmt); + + memset(bind, 0, sizeof(bind)); + + for (i = 0; i < MDEV19838_MAX_PARAM_COUNT; ++i) + { + bind[i].buffer = charvalue; + bind[i].buffer_type = MYSQL_TYPE_STRING; + bind[i].buffer_length = strlen(charvalue) + 1; + bind[i].length = &bind[i].length_value; + bind[i].length_value = bind[i].buffer_length - 1; + } + + for (paramCount = 1; paramCount < MDEV19838_FIELDS_COUNT; ++paramCount) + { + mysql_stmt_attr_set(stmt, STMT_ATTR_PREBIND_PARAMS, ¶mCount); + + rc = mysql_stmt_bind_param(stmt, bind); + check_execute(stmt, rc); + + rc = mariadb_stmt_execute_direct(stmt, "INSERT INTO mdev19838" + "(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11, f12, f13, f14, f15, f16, f17)" + " VALUES " + "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", -1); + + /* Expecting an error */ + DIE_UNLESS(rc != 0); + + mysql_stmt_close(stmt); + stmt = mysql_stmt_init(mysql); + check_stmt(stmt); + } + + paramCount = 0; + mysql_stmt_attr_set(stmt, STMT_ATTR_PREBIND_PARAMS, ¶mCount); + rc = mariadb_stmt_execute_direct(stmt, "INSERT INTO mdev19838(f1)" + " VALUES (?)", -1); + /* Expecting an error */ + DIE_UNLESS(rc != 0); + mysql_stmt_close(stmt); + + stmt = mysql_stmt_init(mysql); + check_stmt(stmt); + /* Correct number of parameters */ + paramCount = MDEV19838_FIELDS_COUNT; + mysql_stmt_attr_set(stmt, STMT_ATTR_PREBIND_PARAMS, ¶mCount); + mysql_stmt_bind_param(stmt, bind); + + rc = mariadb_stmt_execute_direct(stmt, "INSERT INTO mdev19838" + "(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11, f12, f13, f14, f15, f16, f17)" + " VALUES " + "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", -1); + check_execute(stmt, rc); + + /* MYSQL_TYPE_TINY = 1. This parameter byte can be read as "parameters send" flag byte. + Checking that wrong packet is still detected */ + bind[0].buffer_type = MYSQL_TYPE_TINY; + bind[0].length_value = 1; + bind[0].buffer_length = 1; + + for (paramCount = 8; paramCount > 0; --paramCount) + { + mysql_stmt_close(stmt); + stmt = mysql_stmt_init(mysql); + check_stmt(stmt); + + mysql_stmt_attr_set(stmt, STMT_ATTR_PREBIND_PARAMS, ¶mCount); + + rc = mysql_stmt_bind_param(stmt, bind); + check_execute(stmt, rc); + + rc = mariadb_stmt_execute_direct(stmt, "INSERT INTO mdev19838" + "(f1, f2, f3, f4, f5, f6, f7, f8, f9)" + " VALUES " + "(?, ?, ?, ?, ?, ?, ?, ?, ?)", -1); + + /* Expecting an error */ + DIE_UNLESS(rc != 0); + } + + /* Test of query w/out parameters, with parameter sent and not sent */ + for (paramCount = MDEV19838_MAX_PARAM_COUNT; paramCount != (unsigned int)-1; --paramCount) + { + mysql_stmt_close(stmt); + stmt = mysql_stmt_init(mysql); + check_stmt(stmt); + + mysql_stmt_attr_set(stmt, STMT_ATTR_PREBIND_PARAMS, ¶mCount); + + if (paramCount > 0) + { + rc = mysql_stmt_bind_param(stmt, bind); + check_execute(stmt, rc); + } + + rc = mariadb_stmt_execute_direct(stmt, "INSERT INTO mdev19838" + "(f1)" + " VALUES " + "(0x1111111111111111)", -1); + + /* Expecting an error if parameters are sent */ + DIE_UNLESS(rc != 0 || paramCount == 0); + } + + mysql_stmt_close(stmt); + + rc = mysql_query(mysql, "drop table mdev19838"); + myquery(rc); +} + static struct my_tests_st my_tests[]= { { "disable_query_logs", disable_query_logs }, { "test_view_sp_list_fields", test_view_sp_list_fields }, @@ -21152,6 +21299,7 @@ static struct my_tests_st my_tests[]= { #endif { "test_ps_params_in_ctes", test_ps_params_in_ctes }, { "test_explain_meta", test_explain_meta }, + { "test_mdev19838", test_mdev19838 }, { 0, 0 } }; |