diff options
-rw-r--r-- | sql/sql_prepare.cc | 199 | ||||
-rw-r--r-- | tests/mysql_client_test.c | 149 |
2 files changed, 336 insertions, 12 deletions
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 1efb9d713bc..0df2617bb9a 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. */ @@ -1003,11 +1006,73 @@ 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 + + @retval true this parameter is wrong + @retval false this parameter is OK +*/ + +static 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: + case MYSQL_TYPE_NEWDATE: + break; + /* + 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: + 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 *read_pos= *data; - const uint signed_bit= 1 << 15; + DBUG_ENTER("set_conversion_functions"); /* First execute or types altered by the client, setup the @@ -1020,12 +1085,17 @@ 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); setup_one_conversion_function(thd, *it, (uchar) (typecode & 0xff)); (*it)->sync_clones(); } @@ -1035,7 +1105,7 @@ static bool set_conversion_functions(Prepared_statement *stmt, static bool setup_conversion_functions(Prepared_statement *stmt, - uchar **data, uchar *data_end, + uchar **data, bool bulk_protocol= 0) { /* skip null bits */ @@ -1048,7 +1118,7 @@ static bool setup_conversion_functions(Prepared_statement *stmt, if (*read_pos++) //types supplied / first execute { *data= read_pos; - bool res= set_conversion_functions(stmt, data, data_end); + bool res= set_conversion_functions(stmt, data); DBUG_RETURN(res); } *data= read_pos; @@ -3159,11 +3229,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 */ @@ -3219,6 +3298,84 @@ 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 + @param read_bytes need to read types (only with bulk_op) + + @retval true this parameter is wrong + @retval false this parameter is OK +*/ + +static bool +stmt_execute_packet_sanity_check(Prepared_statement *stmt, + uchar *packet, uchar *packet_end, + bool bulk_op, bool direct_exec, + bool read_types) +{ + + DBUG_ASSERT((!read_types) || (read_types && bulk_op)); + if (stmt->param_count > 0) + { + uint packet_length= static_cast<uint>(packet_end - packet); + uint null_bitmap_bytes= (bulk_op ? 0 : (stmt->param_count + 7)/8); + uint min_len_for_param_count = null_bitmap_bytes + + (bulk_op ? 0 : 1); /* sent types byte */ + + if (!bulk_op && packet_length >= min_len_for_param_count) + { + if ((read_types= 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; + } + } + } + + if (read_types) + { + /* 2 bytes per parameter of the type and flags */ + min_len_for_param_count+= 2*stmt->param_count; + } + 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 @@ -3258,6 +3415,24 @@ 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 required 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, read_types)) + { + 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) @@ -4168,7 +4343,7 @@ Prepared_statement::set_parameters(String *expanded_query, { #ifndef EMBEDDED_LIBRARY uchar *null_array= packet; - res= (setup_conversion_functions(this, &packet, packet_end) || + res= (setup_conversion_functions(this, &packet) || set_params(this, null_array, packet, packet_end, expanded_query)); #else /* @@ -4400,7 +4575,7 @@ Prepared_statement::execute_bulk_loop(String *expanded_query, #ifndef EMBEDDED_LIBRARY if (read_types && - set_conversion_functions(this, &packet, packet_end)) + set_conversion_functions(this, &packet)) #else // bulk parameters are not supported for embedded, so it will an error #endif diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 9a90862e93f..058168eedd5 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -19897,6 +19897,152 @@ static void test_ps_params_in_ctes() } +#ifndef EMBEDDED_LIBRARY +#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); +} +#endif // EMBEDDED_LIBRARY + static struct my_tests_st my_tests[]= { { "disable_query_logs", disable_query_logs }, { "test_view_sp_list_fields", test_view_sp_list_fields }, @@ -20182,6 +20328,9 @@ static struct my_tests_st my_tests[]= { { "test_bulk_replace", test_bulk_replace }, #endif { "test_ps_params_in_ctes", test_ps_params_in_ctes }, +#ifndef EMBEDDED_LIBRARY + { "test_mdev19838", test_mdev19838 }, +#endif { 0, 0 } }; |