Fix for Bug #27944 Filtering THD::client capabilities

The server used to trust blindly information from the client about
its capabilities. During the connection handshake the server sends
information about what it supports and then the client sends back a
set of capabilities which cover all of the server's or less.
Before this changeset the server didn't check whether the flags sent
by the client were valid for the server. For example, if the server
doesn't support compressed protocol but the client does and sends that
bit turned on, the server didn't check it. The change make the server code
less error prone to problems related to the value of THD::client_capabilities.

Clearly there is no vulnerability being fixed but this is a maintainenance
fix to prevent misusage in the future.
parent 6c0dcad8
...@@ -148,6 +148,37 @@ enum enum_server_command ...@@ -148,6 +148,37 @@ enum enum_server_command
#define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30) #define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30)
#define CLIENT_REMEMBER_OPTIONS (1UL << 31) #define CLIENT_REMEMBER_OPTIONS (1UL << 31)
/* Gather all possible capabilites (flags) supported by the server */
#define CLIENT_ALL_FLAGS (CLIENT_LONG_PASSWORD | \
CLIENT_FOUND_ROWS | \
CLIENT_LONG_FLAG | \
CLIENT_CONNECT_WITH_DB | \
CLIENT_NO_SCHEMA | \
CLIENT_COMPRESS | \
CLIENT_ODBC | \
CLIENT_LOCAL_FILES | \
CLIENT_IGNORE_SPACE | \
CLIENT_PROTOCOL_41 | \
CLIENT_INTERACTIVE | \
CLIENT_SSL | \
CLIENT_IGNORE_SIGPIPE | \
CLIENT_TRANSACTIONS | \
CLIENT_RESERVED | \
CLIENT_SECURE_CONNECTION | \
CLIENT_MULTI_STATEMENTS | \
CLIENT_MULTI_RESULTS | \
CLIENT_SSL_VERIFY_SERVER_CERT | \
CLIENT_REMEMBER_OPTIONS)
/*
Switch off the flags that are optional and depending on build flags
If any of the optional flags is supported by the build it will be switched
on before sending to the client during the connection handshake.
*/
#define CLIENT_BASIC_FLAGS (((CLIENT_ALL_FLAGS & ~CLIENT_SSL) \
& ~CLIENT_COMPRESS) \
& ~CLIENT_SSL_VERIFY_SERVER_CERT)
#define SERVER_STATUS_IN_TRANS 1 /* Transaction has started */ #define SERVER_STATUS_IN_TRANS 1 /* Transaction has started */
#define SERVER_STATUS_AUTOCOMMIT 2 /* Server in auto_commit mode */ #define SERVER_STATUS_AUTOCOMMIT 2 /* Server in auto_commit mode */
#define SERVER_MORE_RESULTS_EXISTS 8 /* Multi query - next query exists */ #define SERVER_MORE_RESULTS_EXISTS 8 /* Multi query - next query exists */
......
...@@ -700,20 +700,24 @@ static int check_connection(THD *thd) ...@@ -700,20 +700,24 @@ static int check_connection(THD *thd)
bzero((char*) &thd->remote, sizeof(thd->remote)); bzero((char*) &thd->remote, sizeof(thd->remote));
} }
vio_keepalive(net->vio, TRUE); vio_keepalive(net->vio, TRUE);
ulong server_capabilites;
{ {
/* buff[] needs to big enough to hold the server_version variable */ /* buff[] needs to big enough to hold the server_version variable */
char buff[SERVER_VERSION_LENGTH + SCRAMBLE_LENGTH + 64]; char buff[SERVER_VERSION_LENGTH + SCRAMBLE_LENGTH + 64];
ulong client_flags = (CLIENT_LONG_FLAG | CLIENT_CONNECT_WITH_DB | server_capabilites= CLIENT_BASIC_FLAGS;
CLIENT_PROTOCOL_41 | CLIENT_SECURE_CONNECTION);
if (opt_using_transactions) if (opt_using_transactions)
client_flags|=CLIENT_TRANSACTIONS; server_capabilites|= CLIENT_TRANSACTIONS;
#ifdef HAVE_COMPRESS #ifdef HAVE_COMPRESS
client_flags |= CLIENT_COMPRESS; server_capabilites|= CLIENT_COMPRESS;
#endif /* HAVE_COMPRESS */ #endif /* HAVE_COMPRESS */
#ifdef HAVE_OPENSSL #ifdef HAVE_OPENSSL
if (ssl_acceptor_fd) if (ssl_acceptor_fd)
client_flags |= CLIENT_SSL; /* Wow, SSL is available! */ {
server_capabilites |= CLIENT_SSL; /* Wow, SSL is available! */
server_capabilites |= CLIENT_SSL_VERIFY_SERVER_CERT;
}
#endif /* HAVE_OPENSSL */ #endif /* HAVE_OPENSSL */
end= strnmov(buff, server_version, SERVER_VERSION_LENGTH) + 1; end= strnmov(buff, server_version, SERVER_VERSION_LENGTH) + 1;
...@@ -732,7 +736,7 @@ static int check_connection(THD *thd) ...@@ -732,7 +736,7 @@ static int check_connection(THD *thd)
*/ */
end= strmake(end, thd->scramble, SCRAMBLE_LENGTH_323) + 1; end= strmake(end, thd->scramble, SCRAMBLE_LENGTH_323) + 1;
int2store(end, client_flags); int2store(end, server_capabilites);
/* write server characteristics: up to 16 bytes allowed */ /* write server characteristics: up to 16 bytes allowed */
end[2]=(char) default_charset_info->number; end[2]=(char) default_charset_info->number;
int2store(end+3, thd->server_status); int2store(end+3, thd->server_status);
...@@ -762,7 +766,7 @@ static int check_connection(THD *thd) ...@@ -762,7 +766,7 @@ static int check_connection(THD *thd)
if (thd->packet.alloc(thd->variables.net_buffer_length)) if (thd->packet.alloc(thd->variables.net_buffer_length))
return 1; /* The error is set by alloc(). */ return 1; /* The error is set by alloc(). */
thd->client_capabilities=uint2korr(net->read_pos); thd->client_capabilities= uint2korr(net->read_pos);
if (thd->client_capabilities & CLIENT_PROTOCOL_41) if (thd->client_capabilities & CLIENT_PROTOCOL_41)
{ {
thd->client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16; thd->client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16;
...@@ -777,6 +781,11 @@ static int check_connection(THD *thd) ...@@ -777,6 +781,11 @@ static int check_connection(THD *thd)
thd->max_client_packet_length= uint3korr(net->read_pos+2); thd->max_client_packet_length= uint3korr(net->read_pos+2);
end= (char*) net->read_pos+5; end= (char*) net->read_pos+5;
} }
/*
Disable those bits which are not supported by the server.
This is a precautionary measure, if the client lies. See Bug#27944.
*/
thd->client_capabilities&= server_capabilites;
if (thd->client_capabilities & CLIENT_IGNORE_SPACE) if (thd->client_capabilities & CLIENT_IGNORE_SPACE)
thd->variables.sql_mode|= MODE_IGNORE_SPACE; thd->variables.sql_mode|= MODE_IGNORE_SPACE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment