Description:
In MySQL 8.0.1 the code that executes prepared statements was modified. This modifications has a bug that can lead to a read 1 byte past a allocated/initialized buffer.
In case this byte past the buffer is not zero memory past the allocated buffer can be read by the server and misinterpreted as information sent by the client.
How to repeat:
To achieve this one needs to send specially crafter COM_STMT_EXECUTE packet.
In my case this is easy if PHP's mysqlnd library is slighty patched with the following patch:
diff --git a/ext/mysqlnd/mysqlnd_ps_codec.c b/ext/mysqlnd/mysqlnd_ps_codec.c
index c61bb30edb..fa3246d970 100644
-------------------------------------cut here-----
--- a/ext/mysqlnd/mysqlnd_ps_codec.c
+++ b/ext/mysqlnd/mysqlnd_ps_codec.c
@@ -852,7 +852,7 @@ mysqlnd_stmt_execute_store_params(MYSQLND_STMT * s, zend_uchar **buf, zend_uchar
memset(*p, 0, null_count);
*p += null_count;
}
-
+ DBG_RETURN(PASS);
/* 1. Store type information */
/*
check if need to send the types even if stmt->send_types_to_server is 0. This is because
-------------------------------------cut here-----
Follows the code that is faulty:
/* Then comes the null bits */
const uint null_bits_packet_len= (param_count + 7) / 8;
if (packet_left < null_bits_packet_len)
goto malformed;
unsigned char *null_bits= read_pos;
read_pos += null_bits_packet_len;
packet_left-= null_bits_packet_len;
PS_PARAM *params= data->com_stmt_execute.parameters;
/* Then comes the types byte. If set, new types are provided */
bool has_new_types= static_cast<bool>(*read_pos++);
data->com_stmt_execute.has_new_types= has_new_types;
if (has_new_types)
The assignment to has_new_types read from *read_pos without a previous check whether packet_left is non-zero. This read is unchecked. If the client sends a packet, which is shorter than expected the read from read_pos will be past the buffer passed to the function. In case the byte past the buffer is not initialized to 0 then the code will go into this block:
DBUG_PRINT("info", ("Types provided"));
--packet_left;
for (uint i= 0; i < param_count; ++i)
{
if (packet_left < 2)
goto malformed;
ushort type_code= sint2korr(read_pos);
read_pos+= 2;
packet_left-= 2;
const uint signed_bit= 1 << 15;
params[i].type=
static_cast<enum enum_field_types>(type_code & ~signed_bit);
params[i].unsigned_type= static_cast<bool>(type_code & signed_bit);
DBUG_PRINT("info", ("type=%u", (uint) params[i].type));
DBUG_PRINT("info", ("flags=%u", (uint) params[i].unsigned_type));
}
}
Here, you can see that packet_left is decremented by 1. In the case of the specially crafted packet packet_length was 0 before the decrementation. Because packet_left is an unsigned long, the variable will rotate and the value will be 2^64-1 (on a 64bit box) and so the protection that packet_length provides is gone. The code will read and read past the buffer. With a prepared statement that has a lot of variables a lot of memory past the network buffer will be read.
Suggested fix:
Don't read before checking whether there is more data in the buffer.
Using counters for this kind of checking is error prone and as it can be seen has led to a bug. A better approach is to use pointer arithmetic which includes the pointer to the current byte in the buffer. This is a single source of truth, anything else is redundant info and can get out of sync.
Description: In MySQL 8.0.1 the code that executes prepared statements was modified. This modifications has a bug that can lead to a read 1 byte past a allocated/initialized buffer. In case this byte past the buffer is not zero memory past the allocated buffer can be read by the server and misinterpreted as information sent by the client. How to repeat: To achieve this one needs to send specially crafter COM_STMT_EXECUTE packet. In my case this is easy if PHP's mysqlnd library is slighty patched with the following patch: diff --git a/ext/mysqlnd/mysqlnd_ps_codec.c b/ext/mysqlnd/mysqlnd_ps_codec.c index c61bb30edb..fa3246d970 100644 -------------------------------------cut here----- --- a/ext/mysqlnd/mysqlnd_ps_codec.c +++ b/ext/mysqlnd/mysqlnd_ps_codec.c @@ -852,7 +852,7 @@ mysqlnd_stmt_execute_store_params(MYSQLND_STMT * s, zend_uchar **buf, zend_uchar memset(*p, 0, null_count); *p += null_count; } - + DBG_RETURN(PASS); /* 1. Store type information */ /* check if need to send the types even if stmt->send_types_to_server is 0. This is because -------------------------------------cut here----- Follows the code that is faulty: /* Then comes the null bits */ const uint null_bits_packet_len= (param_count + 7) / 8; if (packet_left < null_bits_packet_len) goto malformed; unsigned char *null_bits= read_pos; read_pos += null_bits_packet_len; packet_left-= null_bits_packet_len; PS_PARAM *params= data->com_stmt_execute.parameters; /* Then comes the types byte. If set, new types are provided */ bool has_new_types= static_cast<bool>(*read_pos++); data->com_stmt_execute.has_new_types= has_new_types; if (has_new_types) The assignment to has_new_types read from *read_pos without a previous check whether packet_left is non-zero. This read is unchecked. If the client sends a packet, which is shorter than expected the read from read_pos will be past the buffer passed to the function. In case the byte past the buffer is not initialized to 0 then the code will go into this block: DBUG_PRINT("info", ("Types provided")); --packet_left; for (uint i= 0; i < param_count; ++i) { if (packet_left < 2) goto malformed; ushort type_code= sint2korr(read_pos); read_pos+= 2; packet_left-= 2; const uint signed_bit= 1 << 15; params[i].type= static_cast<enum enum_field_types>(type_code & ~signed_bit); params[i].unsigned_type= static_cast<bool>(type_code & signed_bit); DBUG_PRINT("info", ("type=%u", (uint) params[i].type)); DBUG_PRINT("info", ("flags=%u", (uint) params[i].unsigned_type)); } } Here, you can see that packet_left is decremented by 1. In the case of the specially crafted packet packet_length was 0 before the decrementation. Because packet_left is an unsigned long, the variable will rotate and the value will be 2^64-1 (on a 64bit box) and so the protection that packet_length provides is gone. The code will read and read past the buffer. With a prepared statement that has a lot of variables a lot of memory past the network buffer will be read. Suggested fix: Don't read before checking whether there is more data in the buffer. Using counters for this kind of checking is error prone and as it can be seen has led to a bug. A better approach is to use pointer arithmetic which includes the pointer to the current byte in the buffer. This is a single source of truth, anything else is redundant info and can get out of sync.