From 9b68760fd61869626808bf47ac75a9024ea662b7 Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Mon, 30 May 2011 07:42:30 -0300 Subject: Bug#12563279: REGRESSION IN HANDLING PRE-4.1 AUTHENTICATION PACKET The problem is that clients implementing the 4.0 version of the protocol (that is, mysql-4.0) do not null terminate a string at the end of the authentication packet. These clients denote the end of the string with the end of the packet. Although this goes against the documented (see MySQL Internals ClientServer Protocol wiki) description of the protocol, these old clients still need to be supported. The solution is to support the documented and actual behavior of the clients. If a client is using the pre-4.1 version of the protocol, the end of a string in the authentication packet can either be denoted with a null character or by the end of the packet. This restores backwards compatibility with old clients implementing either the documented or actual behavior. sql/password.c: The scrambled message, as provided by the user, might not be properly null terminated. If this is the case, uninitialized memory past the end of the buffer could theoretically be accessed. To ensure that this is never the case, copy the scrambled message over to a null terminated auxiliar buffer. sql/sql_connect.cc: Use different execution paths to read strings depending on the protocol being used. If version 4.0 of the protocol is used, end of string can be denoted with a NUL character or by the end of the packet. If there are not enough bytes left after the current position of the buffer to satisfy the current string, the string is considered to be empty. This is required because old clients do not send the password string field if the password is empty. --- sql/password.c | 32 +++++++------- sql/sql_connect.cc | 121 +++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 115 insertions(+), 38 deletions(-) (limited to 'sql') diff --git a/sql/password.c b/sql/password.c index 9204c660b77..29a501986f4 100644 --- a/sql/password.c +++ b/sql/password.c @@ -204,21 +204,16 @@ void scramble_323(char *to, const char *message, const char *password) } -/* - Check scrambled message - Used in pre 4.1 password handling - SYNOPSIS - check_scramble_323() - scrambled scrambled message to check. - message original random message which was used for scrambling; must - be exactly SCRAMBLED_LENGTH_323 bytes long and - NULL-terminated. - hash_pass password which should be used for scrambling - All params are IN. +/** + Check scrambled message. Used in pre 4.1 password handling. - RETURN VALUE - 0 - password correct - !0 - password invalid + @param scrambled Scrambled message to check. + @param message Original random message which was used for scrambling. + @param hash_pass Password which should be used for scrambling. + + @remark scrambled and message must be SCRAMBLED_LENGTH_323 bytes long. + + @return FALSE if password is correct, TRUE otherwise. */ my_bool @@ -227,9 +222,16 @@ check_scramble_323(const char *scrambled, const char *message, { struct rand_struct rand_st; ulong hash_message[2]; - char buff[16],*to,extra; /* Big enough for check */ + /* Big enough for checks. */ + char buff[16], scrambled_buff[SCRAMBLE_LENGTH_323 + 1]; + char *to, extra; const char *pos; + /* Ensure that the scrambled message is null-terminated. */ + memcpy(scrambled_buff, scrambled, SCRAMBLE_LENGTH_323); + scrambled_buff[SCRAMBLE_LENGTH_323]= '\0'; + scrambled= scrambled_buff; + hash_password(hash_message, message, SCRAMBLE_LENGTH_323); randominit(&rand_st,hash_pass[0] ^ hash_message[0], hash_pass[1] ^ hash_message[1]); diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 1b27efdd39a..26600584256 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -653,14 +653,21 @@ bool init_new_connection_handler_thread() } #ifndef EMBEDDED_LIBRARY + +/** Get a string according to the protocol of the underlying buffer. */ +typedef char * (*get_proto_string_func_t) (char **, size_t *, size_t *); + /** - Get a null character terminated string from a user-supplied buffer. + Get a string formatted according to the 4.1 version of the MySQL protocol. - @param buffer[in, out] Pointer to the buffer to be scanned. + @param buffer[in, out] Pointer to the user-supplied buffer to be scanned. @param max_bytes_available[in, out] Limit the bytes to scan. @param string_length[out] The number of characters scanned not including the null character. + @remark Strings are always null character terminated in this version of the + protocol. + @remark The string_length does not include the terminating null character. However, after the call, the buffer is increased by string_length+1 bytes, beyond the null character if there still available bytes to @@ -671,9 +678,9 @@ bool init_new_connection_handler_thread() */ static -char *get_null_terminated_string(char **buffer, - size_t *max_bytes_available, - size_t *string_length) +char *get_41_protocol_string(char **buffer, + size_t *max_bytes_available, + size_t *string_length) { char *str= (char *)memchr(*buffer, '\0', *max_bytes_available); @@ -683,7 +690,60 @@ char *get_null_terminated_string(char **buffer, *string_length= (size_t)(str - *buffer); *max_bytes_available-= *string_length + 1; str= *buffer; - *buffer += *string_length + 1; + *buffer += *string_length + 1; + + return str; +} + + +/** + Get a string formatted according to the 4.0 version of the MySQL protocol. + + @param buffer[in, out] Pointer to the user-supplied buffer to be scanned. + @param max_bytes_available[in, out] Limit the bytes to scan. + @param string_length[out] The number of characters scanned not including + the null character. + + @remark If there are not enough bytes left after the current position of + the buffer to satisfy the current string, the string is considered + to be empty and a pointer to empty_c_string is returned. + + @remark A string at the end of the packet is not null terminated. + + @return Pointer to beginning of the string scanned, or a pointer to a empty + string. +*/ +static +char *get_40_protocol_string(char **buffer, + size_t *max_bytes_available, + size_t *string_length) +{ + char *str; + size_t len; + + /* No bytes to scan left, treat string as empty. */ + if ((*max_bytes_available) == 0) + { + *string_length= 0; + return empty_c_string; + } + + str= (char *) memchr(*buffer, '\0', *max_bytes_available); + + /* + If the string was not null terminated by the client, + the remainder of the packet is the string. Otherwise, + advance the buffer past the end of the null terminated + string. + */ + if (str == NULL) + len= *string_length= *max_bytes_available; + else + len= (*string_length= (size_t)(str - *buffer)) + 1; + + str= *buffer; + *buffer+= len; + *max_bytes_available-= len; return str; } @@ -695,7 +755,7 @@ char *get_null_terminated_string(char **buffer, @param buffer[in, out] The buffer to scan; updates position after scan. @param max_bytes_available[in, out] Limit the number of bytes to scan @param string_length[out] Number of characters scanned - + @remark In case the length is zero, then the total size of the string is considered to be 1 byte; the size byte. @@ -854,7 +914,7 @@ static int check_connection(THD *thd) part at the end of packet. */ end= strmake(end, thd->scramble, SCRAMBLE_LENGTH_323) + 1; - + int2store(end, server_capabilites); /* write server characteristics: up to 16 bytes allowed */ end[2]=(char) default_charset_info->number; @@ -952,7 +1012,20 @@ static int check_connection(THD *thd) if ((thd->client_capabilities & CLIENT_TRANSACTIONS) && opt_using_transactions) net->return_status= &thd->server_status; - + + /* + The 4.0 and 4.1 versions of the protocol differ on how strings + are terminated. In the 4.0 version, if a string is at the end + of the packet, the string is not null terminated. Do not assume + that the returned string is always null terminated. + */ + get_proto_string_func_t get_string; + + if (thd->client_capabilities & CLIENT_PROTOCOL_41) + get_string= get_41_protocol_string; + else + get_string= get_40_protocol_string; + /* In order to safely scan a head for '\0' string terminators we must keep track of how many bytes remain in the allocated @@ -961,8 +1034,7 @@ static int check_connection(THD *thd) size_t bytes_remaining_in_packet= pkt_len - (end - (char *)net->read_pos); size_t user_len; - char *user= get_null_terminated_string(&end, &bytes_remaining_in_packet, - &user_len); + char *user= get_string(&end, &bytes_remaining_in_packet, &user_len); if (user == NULL) { inc_host_errors(&thd->remote.sin_addr); @@ -991,8 +1063,7 @@ static int check_connection(THD *thd) /* Old passwords are zero terminated strings. */ - passwd= get_null_terminated_string(&end, &bytes_remaining_in_packet, - &passwd_len); + passwd= get_string(&end, &bytes_remaining_in_packet, &passwd_len); } if (passwd == NULL) @@ -1007,8 +1078,7 @@ static int check_connection(THD *thd) if (thd->client_capabilities & CLIENT_CONNECT_WITH_DB) { - db= get_null_terminated_string(&end, &bytes_remaining_in_packet, - &db_len); + db= get_string(&end, &bytes_remaining_in_packet, &db_len); if (db == NULL) { inc_host_errors(&thd->remote.sin_addr); @@ -1021,19 +1091,24 @@ static int check_connection(THD *thd) char user_buff[USERNAME_LENGTH + 1]; // buffer to store user in utf8 uint dummy_errors; - /* Since 4.1 all database names are stored in utf8 */ + /* + Copy and convert the user and database names to the character set used + by the server. Since 4.1 all database names are stored in UTF-8. Also, + ensure that the names are properly null-terminated as this is relied + upon later. + */ if (db) { - db_buff[copy_and_convert(db_buff, sizeof(db_buff)-1, - system_charset_info, - db, db_len, - thd->charset(), &dummy_errors)]= 0; + db_len= copy_and_convert(db_buff, sizeof(db_buff)-1, system_charset_info, + db, db_len, thd->charset(), &dummy_errors); + db_buff[db_len]= '\0'; db= db_buff; } - user_buff[user_len= copy_and_convert(user_buff, sizeof(user_buff)-1, - system_charset_info, user, user_len, - thd->charset(), &dummy_errors)]= '\0'; + user_len= copy_and_convert(user_buff, sizeof(user_buff)-1, + system_charset_info, user, user_len, + thd->charset(), &dummy_errors); + user_buff[user_len]= '\0'; user= user_buff; /* If username starts and ends in "'", chop them off */ -- cgit v1.2.1