#At file:///Users/shulga/projects/mysql/5.1-bugteam-bug56976/ based on revid:mats.kindahl@stripped
3512 Dmitry Shulga 2010-12-30
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_allowed_packet constant. When intermediate string
exceeds max_allowed_packet value ER_TOO_LONG_STRING error is emitted.
Incompatible change: It is no longer possible to use
mysql_send_long_data() API to provide values for BLOBs with length
more than 1G.
@ sql/item.cc
Added call to my_error when accumulated string exceeds
max_allowed_packet value. my_error() calls handler for
error ER_TOO_LONG_STRING 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/item.h
Added argument of type THD* to declaration of set_longdata().
@ sql/sql_prepare.cc
Added error handler class Set_longdata_error_handler. This
handler is installed inside mysql_stmt_get_longdata()
before call to param->set_longdata and deinstalled after
return from it.
Sourcecode snippet that makes checking for statement's state
during statement execution moved from Prepared_statement::execute()
to Prepared_statement::execute_loop() in order not to call
set_parameters() when statement has been failed during
set_long_data() execution. If it didn't do then call to
set_parameters would failed.
@ tests/mysql_client_test.c
It was added testcase for the bug #56976.
modified:
sql/item.cc
sql/item.h
sql/sql_prepare.cc
tests/mysql_client_test.c
=== modified file 'sql/item.cc'
--- a/sql/item.cc 2010-11-18 13:11:18 +0000
+++ b/sql/item.cc 2010-12-30 07:03:56 +0000
@@ -2738,7 +2738,7 @@ bool Item_param::set_str(const char *str
}
-bool Item_param::set_longdata(const char *str, ulong length)
+bool Item_param::set_longdata(THD *thd, const char *str, ulong length)
{
DBUG_ENTER("Item_param::set_longdata");
@@ -2751,6 +2751,9 @@ bool Item_param::set_longdata(const char
(here), and first have to concatenate all pieces together,
write query to the binary log and only then perform conversion.
*/
+ if (str_value.length() + length > thd->variables.max_allowed_packet)
+ DBUG_RETURN(my_error(ER_TOO_LONG_STRING, MYF(0)));
+
if (str_value.append(str, length, &my_charset_bin))
DBUG_RETURN(TRUE);
state= LONG_DATA_VALUE;
=== modified file 'sql/item.h'
--- a/sql/item.h 2010-07-30 13:35:06 +0000
+++ b/sql/item.h 2010-12-30 07:03:56 +0000
@@ -1687,7 +1687,7 @@ public:
void set_double(double i);
void set_decimal(const char *str, ulong length);
bool set_str(const char *str, ulong length);
- bool set_longdata(const char *str, ulong length);
+ bool set_longdata(THD *thd, const char *str, ulong length);
void set_time(MYSQL_TIME *tm, timestamp_type type, uint32 max_length_arg);
bool set_from_user_var(THD *thd, const user_var_entry *entry);
void reset();
=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc 2010-11-03 10:24:47 +0000
+++ b/sql/sql_prepare.cc 2010-12-30 07:03:56 +0000
@@ -95,6 +95,7 @@ When one supplies long data for a placeh
#else
#include <mysql_com.h>
#endif
+#include <mysys_err.h>
/**
A result class used to send cursor rows using the binary protocol.
@@ -2743,6 +2744,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)
+ { }
+
+public:
+ bool handle_error(uint sql_errno,
+ const char *message,
+ MYSQL_ERROR::enum_warning_level level,
+ THD *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.
@@ -2799,16 +2826,14 @@ void mysql_stmt_get_longdata(THD *thd, c
param= stmt->param_array[param_number];
+ Set_longdata_error_handler err_handler(stmt);
+ thd->push_internal_handler(&err_handler);
#ifndef EMBEDDED_LIBRARY
- if (param->set_longdata(packet, (ulong) (packet_end - packet)))
+ param->set_longdata(thd, packet, (ulong) (packet_end - packet));
#else
- if (param->set_longdata(thd->extra_data, thd->extra_length))
+ param->set_longdata(thd, 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);
@@ -3269,6 +3294,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;
@@ -3509,12 +3541,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-11-26 12:51:48 +0000
+++ b/tests/mysql_client_test.c 2010-12-30 07:03:56 +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);
+ memset(long_buffer, 'a', packet_len);
+
+ for (i= 0; i < dos_len / packet_len; i++)
+ {
+ rc= mysql_stmt_send_long_data(stmt, 0, long_buffer, packet_len);
+ check_execute(stmt, rc);
+ }
+
+ rc= mysql_stmt_execute(stmt);
+
+ DIE_UNLESS(rc && mysql_stmt_errno(stmt) == ER_TOO_LONG_STRING);
+
+ mysql_stmt_close(stmt);
+
+ DBUG_VOID_RETURN;
+}
+
+
+/*
Read and parse arguments and MySQL options from my.cnf
*/
@@ -18724,6 +18772,7 @@ static struct my_tests_st my_tests[]= {
{ "test_bug42373", test_bug42373 },
{ "test_bug54041", test_bug54041 },
{ "test_bug47485", test_bug47485 },
+ { "test_bug56976", test_bug56976 },
{ 0, 0 }
};
Attachment: [text/bzr-bundle] bzr/dmitry.shulga@oracle.com-20101230070356-v2y9i4x0btmkynck.bundle
| Thread |
|---|
| • bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3512) Bug#56976 | Dmitry Shulga | 30 Dec |