From e9c458865950d7667b081039adb2aa6628d37928 Mon Sep 17 00:00:00 2001
From: David Goulet <dgoulet@riseup.net>
Date: Fri, 17 Feb 2017 14:00:23 -0500
Subject: [PATCH] Remove buggy use of trees_password_fd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Tomasz Miąsko reported multiple issues with the trees_read_line_fd using
the trees_password_fd field. It is currently unused which used to be
used by the unit tests and dovadm in the Posteo scrambler plugin.
The issues are:
In trees_read_line_fd(), this check is bad:
if (bytes_read > MAXIMAL_PASSWORD_LENGTH)
Currently it is incorrect because when "bytes_read >
MAXIMAL_PASSWORD_LENGTH" is true, then buffer capacity is already
exceeded (or just right if you take into account one byte slack
allocated in str_new for terminating null). Moreover, the buffer
will be overrun by one more byte in "pointer[0] = 0;" after leaving
the loop.
Reported-by: Tomasz Miąsko <tomasz.miasko@gmail.com>
Signed-off-by: David Goulet <dgoulet@riseup.net>
---
src/trees-common.c | 29 -----------------------------
src/trees-common.h | 1 -
src/trees-plugin.c | 6 +-----
3 files changed, 1 insertion(+), 35 deletions(-)
diff --git a/src/trees-common.c b/src/trees-common.c
index 6f92712..5698d4e 100644
--- a/src/trees-common.c
+++ b/src/trees-common.c
@@ -46,35 +46,6 @@ trees_initialize(void)
return 0;
}
-const char *
-trees_read_line_fd(pool_t pool, int fd)
-{
- string_t *buffer = str_new(pool, MAXIMAL_PASSWORD_LENGTH);
- char *result = str_c_modifiable(buffer);
- char *pointer = result;
-
- ssize_t read_result = read(fd, pointer, 1);
- unsigned int bytes_read = 0;
- while (read_result != -1 && pointer[0] != '\n') {
- pointer++;
- bytes_read++;
-
- if (bytes_read > MAXIMAL_PASSWORD_LENGTH) {
- i_error("error reading form fd %d: password too long", fd);
- break;
- }
-
- read_result = read(fd, pointer, 1);
- }
-
- pointer[0] = 0;
-
- if (read_result == -1)
- i_error("error reading from fd %d: %s (%d)", fd, strerror(errno), errno);
-
- return result;
-}
-
#ifdef DEBUG_STREAMS
void
diff --git a/src/trees-common.h b/src/trees-common.h
index d8ca434..821883b 100644
--- a/src/trees-common.h
+++ b/src/trees-common.h
@@ -37,7 +37,6 @@
/* Aligns with the docevot default buffer size. */
#define CHUNK_SIZE 8192
#define ENCRYPTED_CHUNK_SIZE (crypto_box_SEALBYTES + CHUNK_SIZE)
-#define MAXIMAL_PASSWORD_LENGTH 256
#define MAX_ISTREAM_BUFFER_SIZE (ENCRYPTED_CHUNK_SIZE * 2)
#define MIN(a,b) \
diff --git a/src/trees-plugin.c b/src/trees-plugin.c
index 8fe131e..238d03f 100644
--- a/src/trees-plugin.c
+++ b/src/trees-plugin.c
@@ -135,7 +135,7 @@ static int
trees_get_private_key(struct mail_user *user,
struct trees_user *suser)
{
- int have_salt, password_fd;
+ int have_salt;
unsigned long long opslimit, memlimit;
unsigned char pw_salt[crypto_pwhash_SALTBYTES];
unsigned char sk_nonce[crypto_secretbox_NONCEBYTES];
@@ -149,10 +149,6 @@ trees_get_private_key(struct mail_user *user,
/* Get the user password that we'll use to . */
password = trees_get_string_setting(user, "trees_password");
- password_fd = trees_get_integer_setting(user, "trees_password_fd");
- if (password == NULL && password_fd >= 0) {
- password = trees_read_line_fd(user->pool, password_fd);
- }
/* No password means that we are receiving email and have no access to the
* user private data so stop now. */
--
GitLab