Fix a buffer overflow in password encryption, and allow arbitrary password length.

Fixes bug #4033 - Claws Mail crashes [malloc(): memory
corruption] while trying to save account password greater
than 136 chars

diff --git a/doc/src/password_encryption.txt b/doc/src/password_encryption.txt
index 77464993f18ecab36ea71328f64afc2c1888209b..f618378d3e2349e8b277053f7bcfa8d3f1e0c0ae 100644 (file)
--- a/doc/src/password_encryption.txt
+++ b/doc/src/password_encryption.txt
@@ -12,9 +12,18 @@ IV (initialization vector) for the cipher is filled with random bytes.
 
 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.

diff --git a/src/password.c b/src/password.c
index 9e66a92253fca1acbc2e04bad8251197c143f10f..8f764e3467d1a57207439c7e07651a619db7ee32 100644 (file)
--- a/src/password.c
+++ b/src/password.c
@@ -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;
-       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;
 
@@ -353,10 +353,18 @@ gchar *password_encrypt_gnutls(const gchar *password,
                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. */
-       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);
@@ -368,10 +376,10 @@ gchar *password_encrypt_gnutls(const gchar *password,
        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);
@@ -389,7 +397,7 @@ gchar *password_encrypt_gnutls(const gchar *password,
 
        /* 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);
@@ -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. */
-       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);