MySQL Bugs: #68339: ENCRYPT() with empty string for password doesn't work as system crypt()

Bug #68339	ENCRYPT() with empty string for password doesn't work as system crypt()
Submitted:	11 Feb 2013 17:09	Modified:	11 Feb 2013 20:04
Reporter:	Mark Zealey	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: DML	Severity:	S3 (Non-critical)
Version:	5.1, 5.5, 5.6	OS:	Linux
Assigned to:		CPU Architecture:	Any

Description:
When I pass a blank password to ENCRYPT() function, rather than the standard crypt() library's return I get the blank string.

How to repeat:
mysql> select encrypt('', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0');
+-------------------------------------------------------------------------------------------------------------------+
| encrypt('', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0') |
+-------------------------------------------------------------------------------------------------------------------+
|                                                                                                                   |
+-------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

Would expect it to return as the libc function:

$ perl -le 'print crypt("", "\$6\$hejzDRlp\$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0")';
$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0

Suggested fix:
Make ENCRYPT() with a blank password mirror the system crypt() function, OR update documentation explaining this corner case.

Hi Mark,

Thank you for your comments!

Here's the relevant info:

mysql> select version();
+------------+
| version()  |
+------------+
| 5.6.10-log |
+------------+
1 row in set (0.02 sec)

mysql> select encrypt('', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0');
+-------------------------------------------------------------------------------------------------------------------+
| encrypt('', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0') |
+-------------------------------------------------------------------------------------------------------------------+
|                                                                                                                   |
+-------------------------------------------------------------------------------------------------------------------+
1 row in set (0.03 sec)

mysql> select encrypt('test', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0');
+-----------------------------------------------------------------------------------------------------------------------+
| encrypt('test', '$6$hejzDRlp$reYyLLxqx6EDFaLm9odeQisc8iegHYeou9fojHg6t1U7mB9xom96DPlQLX2qSk.nhx/mrf1fBqGRZjxkbFNti0') |
+-----------------------------------------------------------------------------------------------------------------------+
| $6$hejzDRlp$QDvJa94d6HVmz43LZBJux5wIE8jfbRqiF577H47sW0xg6WEZ/33UPAtczlkQh48OETFqjXiSV5HfZrvTAFY2A1                    |
+-----------------------------------------------------------------------------------------------------------------------+
1 row in set (0.02 sec)

String *Item_func_encrypt::val_str(String *str)
{
  DBUG_ASSERT(fixed == 1);
  String *res  =args[0]->val_str(str);

#ifdef HAVE_CRYPT
  char salt[3],*salt_ptr;
  if ((null_value=args[0]->null_value))
    return 0;
  if (res->length() == 0)
    return make_empty_result();
  if (arg_count == 1)
  {                                     // generate random salt
    time_t timestamp=current_thd->query_start();
    salt[0] = bin_to_ascii( (ulong) timestamp & 0x3f);
    salt[1] = bin_to_ascii(( (ulong) timestamp >> 5) & 0x3f);
    salt[2] = 0;
    salt_ptr=salt;
  }
  else
  {                                     // obtain salt from the first two bytes
    String *salt_str=args[1]->val_str(&tmp_value);
    if ((null_value= (args[1]->null_value || salt_str->length() < 2)))
      return 0;
    salt_ptr= salt_str->c_ptr_safe();
  }
  mysql_mutex_lock(&LOCK_crypt);
  char *tmp= crypt(res->c_ptr_safe(),salt_ptr);
  if (!tmp)
  {
    mysql_mutex_unlock(&LOCK_crypt);
    null_value= 1;
    return 0;
  }
  str->set(tmp, (uint) strlen(tmp), &my_charset_bin);
  str->copy();
  mysql_mutex_unlock(&LOCK_crypt);
  return str;
#else
  null_value=1;
  return 0;
#endif  /* HAVE_CRYPT */
}

So as you can see, we do handle an empty string specially:
  if (res->length() == 0)
    return make_empty_result();

Did that cause any problems for you? Or potential problems?

Do you see any reason not to simply note this on the manual page, here?
  https://dev.mysql.com/doc/refman/5.6/en/encryption-functions.html#function_encrypt

Is there any specific text that you feel would be most helpful there?

Best Regards,

Matt

To be honest it does cause a bit of a problem as it's not uniform - I was assuming that if I run crypt(string, salt) as a system call or as an ENCRYPT() call it should return the same string as per manual:

"Encrypts str using the Unix crypt() system call and returns a binary string."

However it doesn't do this in this case which seems a bit strange. As I say, making a note in the manual would be OK but I'd prefer to see it fixed - I'm not sure why the conditional is even needed there as an empty string is perfectly valid for all the system crypt() functions that I've ever seen.

It's also an interesting case for password checking.. Your standard password check function would be IF( ENCRYPT( input, hashed_pw ) = hashed_pw, ... ) but in the case where the password & therefore hashed password are '' and someone tries an input password different from that it would result in a DES-crypt string with randomized salt... Anyway, I guess to avoid breaking backwards compatibility in this function you could say if (input == "" && salt == "") (ie password verification) then return "", otherwise crypt() the string properly. This would mean all new passwords were generated correctly but password verifications would still work.

Hi Mark,

I agree, I don't see a compelling reason to deviate from the standard that could reasonably be expected here.

I'll go ahead and set this to verified and copy it over to the development team.

Thank you for the helpful bug report!

Best Regards,

Matt