Fix a buffer overflow in password encryption, and allow arbitrary password length.
authorAndrej Kacian <ticho@claws-mail.org>
Sat, 9 Jun 2018 22:38:42 +0000 (00:38 +0200)
committerAndrej Kacian <ticho@claws-mail.org>
Sat, 9 Jun 2018 22:53:20 +0000 (00:53 +0200)
Fixes bug #4033 - Claws Mail crashes [malloc(): memory
corruption] while trying to save account password greater
than 136 chars

doc/src/password_encryption.txt
src/password.c

index 7746499..f618378 100644 (file)
@@ -12,9 +12,18 @@ IV (initialization vector) for the cipher is filled with random bytes.
 
 Encryption
 ----------
 
 Encryption
 ----------
-We prepare a buffer 128+blocksize bytes long, with one block of random
-data at the beginning, followed by the password we want to encrypt (in
-UTF-8), rest is padded with zero bytes.
+We prepare a buffer long enough to fit the NULL-terminated password string
+plus one cipher block in it, with one block of random data at the beginning,
+followed by the password we want to encrypt (in UTF-8), rest is padded
+with zero bytes.
+
+The minimal buffer size is 128+blocksize, and if the password (including
+the trailing NULL byte) is longer than 128 bytes, the size is increased by
+another 128 bytes until it is long enough to fit the password plus one
+cipher block. This is to make it harder to guess the password length from
+length of the encrypted string. So for example, if the password (again,
+including the trailing NULL byte) is 129 characters long, our buffer will
+be 256+blocksize bytes long.
 
 We encrypt the buffer using the encryption key and IV mentioned above,
 resulting in ciphertext of the same length as the buffer.
 
 We encrypt the buffer using the encryption key and IV mentioned above,
 resulting in ciphertext of the same length as the buffer.
index 9e66a92..8f764e3 100644 (file)
@@ -319,7 +319,7 @@ gchar *password_encrypt_gnutls(const gchar *password,
        gnutls_cipher_algorithm_t algo = GNUTLS_CIPHER_AES_256_CBC;
        gnutls_cipher_hd_t handle;
        gnutls_datum_t key, iv;
        gnutls_cipher_algorithm_t algo = GNUTLS_CIPHER_AES_256_CBC;
        gnutls_cipher_hd_t handle;
        gnutls_datum_t key, iv;
-       int keylen, blocklen, ret;
+       int keylen, blocklen, ret, len, i;
        unsigned char *buf, *encbuf, *base, *output;
        guint rounds = prefs_common_get_prefs()->master_passphrase_pbkdf2_rounds;
 
        unsigned char *buf, *encbuf, *base, *output;
        guint rounds = prefs_common_get_prefs()->master_passphrase_pbkdf2_rounds;
 
@@ -353,10 +353,18 @@ gchar *password_encrypt_gnutls(const gchar *password,
                return NULL;
        }
 
                return NULL;
        }
 
+       /* Find out how big buffer (in multiples of BUFSIZE)
+        * we need to store the password. */
+       i = 1;
+       len = strlen(password);
+       while(len >= i * BUFSIZE)
+               i++;
+       len = i * BUFSIZE;
+
        /* Fill buf with one block of random data, our password, pad the
         * rest with zero bytes. */
        /* Fill buf with one block of random data, our password, pad the
         * rest with zero bytes. */
-       buf = malloc(BUFSIZE + blocklen);
-       memset(buf, 0, BUFSIZE + blocklen);
+       buf = malloc(len + blocklen);
+       memset(buf, 0, len + blocklen);
        if (!get_random_bytes(buf, blocklen)) {
                g_free(buf);
                g_free(key.data);
        if (!get_random_bytes(buf, blocklen)) {
                g_free(buf);
                g_free(key.data);
@@ -368,10 +376,10 @@ gchar *password_encrypt_gnutls(const gchar *password,
        memcpy(buf + blocklen, password, strlen(password));
 
        /* Encrypt into encbuf */
        memcpy(buf + blocklen, password, strlen(password));
 
        /* Encrypt into encbuf */
-       encbuf = malloc(BUFSIZE + blocklen);
-       memset(encbuf, 0, BUFSIZE + blocklen);
-       ret = gnutls_cipher_encrypt2(handle, buf, BUFSIZE + blocklen,
-                       encbuf, BUFSIZE + blocklen);
+       encbuf = malloc(len + blocklen);
+       memset(encbuf, 0, len + blocklen);
+       ret = gnutls_cipher_encrypt2(handle, buf, len + blocklen,
+                       encbuf, len + blocklen);
        if (ret < 0) {
                g_free(key.data);
                g_free(iv.data);
        if (ret < 0) {
                g_free(key.data);
                g_free(iv.data);
@@ -389,7 +397,7 @@ gchar *password_encrypt_gnutls(const gchar *password,
 
        /* And finally prepare the resulting string:
         * "{algorithm,rounds}base64encodedciphertext" */
 
        /* And finally prepare the resulting string:
         * "{algorithm,rounds}base64encodedciphertext" */
-       base = g_base64_encode(encbuf, BUFSIZE + blocklen);
+       base = g_base64_encode(encbuf, len + blocklen);
        g_free(encbuf);
        output = g_strdup_printf("{%s,%d}%s",
                        gnutls_cipher_get_name(algo), rounds, base);
        g_free(encbuf);
        output = g_strdup_printf("{%s,%d}%s",
                        gnutls_cipher_get_name(algo), rounds, base);
@@ -515,7 +523,7 @@ gchar *password_decrypt_gnutls(const gchar *password,
 
        /* 'buf+blocklen' should now be pointing to the plaintext
         * password string. The first block contains random data from the IV. */
 
        /* 'buf+blocklen' should now be pointing to the plaintext
         * password string. The first block contains random data from the IV. */
-       tmp = g_strndup(buf + blocklen, MIN(strlen(buf + blocklen), BUFSIZE));
+       tmp = g_strndup(buf + blocklen, strlen(buf + blocklen));
        memset(buf, 0, len);
        g_free(buf);
 
        memset(buf, 0, len);
        g_free(buf);