Hi Dmitry,
On 2/14/11 10:33 AM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug56976/ based on
> revid:dao-gang.qu@stripped
>
> 3537 Dmitry Shulga 2011-02-14
> Fixed the bug#56976 - Severe Denial Of Service in prepared statements.
>
> The problem was that server didn't check resulting size of prepared
> statement argument which was set using mysql_send_long_data() API.
> By calling mysql_send_long_data() several times it was possible
> to create overly big string and thus force server to allocate
> memory for it. There was no way to limit this allocation.
>
> The solution is to add check for size of result string against
> value of max_long_data_size start-up parameter. When intermediate string
> exceeds max_long_data_size value an appropriate error message is emitted.
>
> For this purpose can be used max_allowed_packet parameter but since its
> value limited by 1GB we use new parameter max_long_data_size.
> max_long_data_size parameter is marked as deprecated and has default
> value that equials to value of max_allowed_packet start-up parameter.
equials -> equals
> In the future start-up parameter max_long_data_size will be
> replaced by max_allowed_packet.
> @ mysql-test/t/variables.test
> Added checking for new start-up parameter max_long_data_size.
> @ sql/item.cc
> Added call to my_message() when accumulated string exceeds
> max_long_data_size value. my_message() calls error handler
> that was installed in mysql_stmt_get_longdata before call
> to Item_param::set_longdata.
>
> The error handler sets to corresponding value the next
> current statement fileds: state, last_error, last_errno.
> @ sql/mysql_priv.h
> Added max_long_data_size variable declaration.
> @ sql/mysqld.cc
> Added support for start-up parameter 'max_long_data_size'.
> This parameter limits size of data which can be sent from
> client to server using mysql_send_long_data() API.
> @ sql/set_var.cc
> Added variable 'max_long_data_size' into list of variables
> displayed by command 'show variables'.
> @ sql/sql_prepare.cc
> Added error handler class Set_longdata_error_handler.
> This handler is used to catch any errors that can be
> generated during execution of Item_param::set_longdata().
>
> Sourcecode snippet that makes checking for statement's state
Sourcecode -> source code
> during statement execution moved from Prepared_statement::execute()
> to Prepared_statement::execute_loop() in order not to call
> set_parameters() when statement has failed during
> set_long_data() execution. If it didn't do then call to
> set_parameters would failed.
> @ tests/mysql_client_test.c
> A testcase for the bug #56976 was added.
>
> modified:
> mysql-test/r/variables.result
> mysql-test/t/variables.test
> sql/item.cc
> sql/mysql_priv.h
> sql/mysqld.cc
> sql/set_var.cc
> sql/sql_prepare.cc
> tests/mysql_client_test.c
> === modified file 'mysql-test/r/variables.result'
> --- a/mysql-test/r/variables.result 2010-11-25 03:11:05 +0000
> +++ b/mysql-test/r/variables.result 2011-02-14 12:33:37 +0000
> @@ -1540,6 +1540,9 @@ ERROR HY000: Cannot drop default keycach
> SET @@global.key_cache_block_size=0;
> Warnings:
> Warning 1292 Truncated incorrect key_cache_block_size value: '0'
> +select @@max_long_data_size;
> +@@max_long_data_size
> +1048576
> SET @@global.max_binlog_cache_size=DEFAULT;
> SET @@global.max_join_size=DEFAULT;
> SET @@global.key_buffer_size=@kbs;
>
> === modified file 'mysql-test/t/variables.test'
> --- a/mysql-test/t/variables.test 2010-11-25 03:11:05 +0000
> +++ b/mysql-test/t/variables.test 2011-02-14 12:33:37 +0000
> @@ -1293,6 +1293,11 @@ SET @@global.max_join_size=0;
> SET @@global.key_buffer_size=0;
> SET @@global.key_cache_block_size=0;
>
> +#
> +# Bug#56976: added new start-up paramaeter
paramaeter -> parameter.
> +#
> +select @@max_long_data_size;
> +
> # cleanup
> SET @@global.max_binlog_cache_size=DEFAULT;
> SET @@global.max_join_size=DEFAULT;
>
[...]
> @@ -8831,6 +8847,14 @@ static int get_options(int *argc,char **
> else
> pool_of_threads_scheduler(&thread_scheduler); /* purecov: tested */
> #endif
> +
> + /*
> + If max_long_data_size is not specified explicitly use
> + value of max_allowed_packet.
> + */
> + if (!max_long_data_size_used)
> + max_long_data_size= global_system_variables.max_allowed_packet;
> +
> return 0;
> }
>
>
> === modified file 'sql/set_var.cc'
> --- a/sql/set_var.cc 2010-12-28 23:47:05 +0000
> +++ b/sql/set_var.cc 2011-02-14 12:33:37 +0000
> @@ -394,6 +394,12 @@ static sys_var_thd_ulong sys_max_seeks_f
> &SV::max_seeks_for_key);
> static sys_var_thd_ulong sys_max_length_for_sort_data(&vars,
> "max_length_for_sort_data",
>
> &SV::max_length_for_sort_data);
> +static sys_var_const sys_max_long_data_size(&vars,
> + "max_long_data_size",
> + OPT_GLOBAL, SHOW_LONG,
> + (uchar*)
> +&max_long_data_size);
Please highlight in the changeset comment that the variable can only be
set at server start.
> #ifndef TO_BE_DELETED /* Alias for max_join_size */
> static sys_var_thd_ha_rows sys_sql_max_join_size(&vars, "sql_max_join_size",
> &SV::max_join_size,
>
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc 2010-12-28 23:47:05 +0000
> +++ b/sql/sql_prepare.cc 2011-02-14 12:33:37 +0000
> @@ -2730,6 +2730,32 @@ void mysql_sql_stmt_close(THD *thd)
> }
> }
>
> +
> +class Set_longdata_error_handler : public Internal_error_handler
> +{
> +public:
> + Set_longdata_error_handler(Prepared_statement *statement)
> + :stmt(statement)
Add space after the colon.
> + { }
> +
> +public:
> + bool handle_error(uint sql_errno,
> + const char *message,
> + MYSQL_ERROR::enum_warning_level level,
> + THD *thd)
Do not name the parameters that are not used. Example:
bool handle_error(uint sql_errno,
const char *message,
MYSQL_ERROR::enum_warning_level,
THD *)
> + {
> + stmt->state= Query_arena::ERROR;
> + stmt->last_errno= sql_errno;
> + strncpy(stmt->last_error, message, MYSQL_ERRMSG_SIZE);
> +
> + return TRUE;
> + }
> +
> +private:
> + Prepared_statement *stmt;
> +};
> +
> +
> /**
> Handle long data in pieces from client.
>
> @@ -2786,16 +2812,19 @@ void mysql_stmt_get_longdata(THD *thd, c
>
> param= stmt->param_array[param_number];
>
> + Set_longdata_error_handler err_handler(stmt);
> + /*
> + Install handler that will catch any errors that can be generated
> + during execution of Item_param::set_longdata() and propagate
> + them to Statement::last_error.
> + */
> + thd->push_internal_handler(&err_handler);
> #ifndef EMBEDDED_LIBRARY
> - if (param->set_longdata(packet, (ulong) (packet_end - packet)))
> + param->set_longdata(packet, (ulong) (packet_end - packet));
> #else
> - if (param->set_longdata(thd->extra_data, thd->extra_length))
> + param->set_longdata(thd->extra_data, thd->extra_length);
> #endif
> - {
> - stmt->state= Query_arena::ERROR;
> - stmt->last_errno= ER_OUTOFMEMORY;
> - sprintf(stmt->last_error, ER(ER_OUTOFMEMORY), 0);
> - }
> + thd->pop_internal_handler();
>
> general_log_print(thd, thd->command, NullS);
>
> @@ -3257,6 +3286,13 @@ Prepared_statement::execute_loop(String
> bool error;
> int reprepare_attempt= 0;
>
> + /* Check if we got an error when sending long data */
> + if (state == Query_arena::ERROR)
> + {
> + my_message(last_errno, last_error, MYF(0));
> + return TRUE;
> + }
> +
> if (set_parameters(expanded_query, packet, packet_end))
> return TRUE;
>
> @@ -3497,12 +3533,6 @@ bool Prepared_statement::execute(String
>
> status_var_increment(thd->status_var.com_stmt_execute);
>
> - /* Check if we got an error when sending long data */
> - if (state == Query_arena::ERROR)
> - {
> - my_message(last_errno, last_error, MYF(0));
> - return TRUE;
> - }
> if (flags& (uint) IS_IN_USE)
> {
> my_error(ER_PS_NO_RECURSION, MYF(0));
>
> === modified file 'tests/mysql_client_test.c'
> --- a/tests/mysql_client_test.c 2010-12-28 23:47:05 +0000
> +++ b/tests/mysql_client_test.c 2011-02-14 12:33:37 +0000
> @@ -18399,6 +18399,54 @@ static void test_bug47485()
>
>
> /*
> + Bug #56976: Severe Denial Of Service in prepared statements
> +*/
> +
> +static void test_bug56976()
> +{
> + MYSQL_STMT *stmt;
> + MYSQL_BIND bind[1];
> + int rc;
> + const char* query = "SELECT LENGTH(?)";
> + char *long_buffer;
> + unsigned long i, packet_len = 256 * 1024L;
> + unsigned long dos_len = 2 * 1024 * 1024L;
> +
> + DBUG_ENTER("test_bug56976");
> + myheader("test_bug56976");
> +
> + stmt= mysql_stmt_init(mysql);
> + check_stmt(stmt);
> +
> + rc= mysql_stmt_prepare(stmt, query, strlen(query));
> + check_execute(stmt, rc);
> +
> + memset(bind, 0, sizeof(bind));
> + bind[0].buffer_type = MYSQL_TYPE_TINY_BLOB;
> +
> + rc= mysql_stmt_bind_param(stmt, bind);
> + check_execute(stmt, rc);
> +
> + long_buffer= malloc(packet_len);
Use my_malloc. Check for failure to allocate. Free it.
> + memset(long_buffer, 'a', packet_len);
> +
[...]
Regards,
Davi