MySQL Bugs: #86357: my_make_scrambled_password() changed behavior, can overflow buffer

Bug #86357	my_make_scrambled_password() changed behavior, can overflow buffer
Submitted:	17 May 2017 14:28	Modified:	19 May 2017 8:23
Reporter:	Andreas Hasenack	Email Updates:
Status:	Won't fix	Impact on me:	None
Category:	MySQL Server: Documentation	Severity:	S3 (Non-critical)
Version:	5.7.18	OS:	Ubuntu
Assigned to:		CPU Architecture:	Any

Description:
In mysql 5.5, 5.6 and 5.7, my_make_scrambled_password() is defined like this:

void my_make_scrambled_password(char *to, const char *password, size_t pass_len)

Its behavior is not the same starting with 5.6, though.

In 5.5 it would write 42 bytes to buf, in the format "*<hexstring>" (https://github.com/mysql/mysql-server/blob/5.5/sql/password.c#L415)

In 5.6 and later, it writes to buf a hash string of length CRYPT_MAX_PASSWORD_SIZE of the format "<salt><string>", and that's much longer than 42 bytes (https://github.com/mysql/mysql-server/blob/5.7/sql/auth/password.c#L186)

There are programs out there that link to libmysqlclient and use my_make_scrambled_password() expecting the behavior in 5.5, that is, they provide a buffer of 42 bytes. Once they link with libmysqlclient20, however, that will overflow buf.

Current code in sql/auth/password.c has confusing docstrings. For example:
*
  Wrapper around my_make_scrambled_password() to maintain client lib ABI
  compatibility.
  In server code usage of my_make_scrambled_password() is preferred to
  avoid strlen().
  SYNOPSIS
    make_scrambled_password()
    buf       OUT buffer of size 2*SHA1_HASH_SIZE + 2 to store hex string
    password  IN  NULL-terminated password string
*/

void make_scrambled_password(char *to, const char *password)
{
  my_make_scrambled_password_sha1(to, password, strlen(password));
}

The docstring says it's wrapping my_make_scrambled_password(), but in fact it's wrapping make_scrambled_password().

The above my_make_scrambled_password_sha1() has the same behavior as mysql's 5.5 my_make_scrambled_password(), but that doesn't help because it's not exported. What is currently being exported is my_make_scrambled_password() that has the new behavior.

Examples of upstream programs that rely (or relied) on the old behavior of my_make_scrambled_password():
- pam_mysql (https://github.com/NigelCunningham/pam-MySQL/blob/master/pam_mysql.c#L2923)
- pure-ftpd, where on this commit they decided to always use an internal implementation exactly because of this problem: https://github.com/jedisct1/pure-ftpd/commit/27443b29320d85352d8b52c0120836843e10c0f9

How to repeat:
Build the following simple .c:
#include <stdio.h>
#include <string.h>

int myfunc(char *password) {
    char buf[42];

    memset(buf, 0, sizeof(buf));
    printf("password=%s, buf=%s\n", password, buf);
    my_make_scrambled_password(buf, password, strlen(password));
    printf("password=%s, buf=%s\n", password, buf);
}

int main(void) {
    char text[] = "password";

    printf("Calling myfunc(%s)\n", text);
    myfunc(text);
    return 0;
}

gcc ff.c -o ff -lmysqlclient -L/usr/lib/x86_64-linux-gnu

Link it with libmysqlclient18 and libmysqlclient20. With 18, it works:
$ ./ff 
Calling myfunc(password)
password=password, buf=
password=password, buf=*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19

With 20, it crashes because of the buffer overflow, and you can also see that the hash it produces is of a different format:
$ ./ff 
Calling myfunc(password)
password=password, buf=
9HR6r]d1password, buf=$5$}
wo1.n $Re.I6ncCQKFuLQH8rAWF89VJVV.K58OQYy0RHVblpZ.
*** stack smashing detected ***: ./ff terminated
Aborted (core dumped)

Suggested fix:
As this is a library, and the damage is done already, I'm not sure if you should change the behavior of my_make_scrambled_password() one more time. Maybe you should just export make_scrambled_password() which has that behavior of producing a "*<hexstring>" instead. Maybe that was even the original intention, but it got confusing with so many functions with similar names and the incorrect one was exported, I don't know.

Hi!

We never maintained backward compatibility on this issue. But, we have had it under control with mysql_upgrade and, internally, with some other data. We are duly maintaining our protocol versions, capability flags and other info, which enables external programs to know EXACTLY what to expect.

However, this might need additional documentation, most likely in our "Internals Manual".

Hence, I am verifying it as a documentation bug.

Thanks for the response.

I want to note that I think there is currently no function exported in the mysqlclient library that creates the same hash as the server SQL PASSWORD() one.

Posted by developer:
 
Hi Andreas,

Just to recap what happened here:

As you say, my_make_scrambled_password changed behavior between 5.5 and 5.6. The old behavior is what is now implemented by my_make_scrambled_password_sha1. In addition, there's make_scrambled_password that is an alias for the old behavior of my_make_scrambled_password (updated to call my_make_scrambled_password_sha1 when the behavior changed). All of these are internal functions that were never meant to be used by third parties, so they were not tracked for ABI breakage. But, unfortunately, the library still exported the symbols, so they ended up being used by a few applications.

So we now have these 3 functions:

my_make_scrambled_password:
  changed behavior between 5.5 and 5.6
my_make_scrambled_password_sha1:
  the 5.5 behavior of my_make_scrambled_password (new in 5.6)
make_scrambled_password:
  the 5.5 behavior (in all versions)

In the initial GA release of 5.7, none of these were exported. During the upgrade from 5.6 to 5.7 in Ubuntu, we got a request to reintroduce one of these (bug #80974), and we added the my_make_scrambled_password function to the export list. In the choice between make_scrambled_password and my_make_scrambled_password, we chose the latter since it uses a more modern hashing algorithm. Since this function was unchanged from 5.6, which was already in Ubuntu, no further inspection of the implementation was done. It was assumed that the 5.6 implementation was what the applications wanted.

The my_make_scrambled_password function hasn't changed behavior after it was added to the ABI in 5.7. The symbol will be removed in a future release.

Posted by developer:
 
MY_MAKE_SCRAMBLED_PASSWORD was never a part of the official C API and is an internal function. These can change and are not bound to the C API version.