From: Timo Sirainen Date: Fri, 6 Sep 2019 13:49:44 +0000 (+0300) Subject: lib, lib-program-client: Fix i_stream_read_memarea() usage X-Git-Tag: 2.3.9~166 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5068b11e594ad7cc1f7cedf2bd9280520e0e534d;p=thirdparty%2Fdovecot%2Fcore.git lib, lib-program-client: Fix i_stream_read_memarea() usage Calling it may change the stream's buffer and free the old buffer. However, some istreams didn't change the buffer if i_stream_read_memarea() returned -2. The old buffer was kept referenced, which could have caused the istream to read garbage or crash due to accessing freed memory. This fixes: * istream-limit * istream-sized * istream-rawlog * program-client-istream * Anything using i_stream_read_copy_from_parent(), which includes: * istream-fs-file * istream-fs-stats * istream-metawrap * http-server-istream * istream-header-filter * istream-mail * istream-failure-at * istream-hash * istream-timeout * istream-try --- diff --git a/src/lib-program-client/program-client-remote.c b/src/lib-program-client/program-client-remote.c index 4ded4c3d32..a9e7c33f0e 100644 --- a/src/lib-program-client/program-client-remote.c +++ b/src/lib-program-client/program-client-remote.c @@ -98,17 +98,15 @@ program_client_istream_read(struct istream_private *stream) ret = -1; } else do { - if ((ret=i_stream_read_memarea(stream->parent)) == -2) { - return -2; /* input buffer full */ - } - - if (ret == 0 || (ret < 0 && !stream->parent->eof)) - break; - + ret = i_stream_read_memarea(stream->parent); stream->istream.stream_errno = stream->parent->stream_errno; stream->buffer = i_stream_get_data(stream->parent, &pos); + if (ret == -2) + return -2; /* input buffer full */ + if (ret == 0 || (ret < 0 && !stream->parent->eof)) + break; if (stream->parent->eof) { /* Check return code at EOF */ diff --git a/src/lib/istream-limit.c b/src/lib/istream-limit.c index 35fd68ff26..53c035218b 100644 --- a/src/lib/istream-limit.c +++ b/src/lib/istream-limit.c @@ -46,13 +46,13 @@ static ssize_t i_stream_limit_read(struct istream_private *stream) if (pos > stream->pos) ret = 0; else do { - if ((ret = i_stream_read_memarea(stream->parent)) == -2) - return -2; - + ret = i_stream_read_memarea(stream->parent); stream->istream.stream_errno = stream->parent->stream_errno; stream->istream.eof = stream->parent->eof; stream->buffer = i_stream_get_data(stream->parent, &pos); } while (pos <= stream->pos && ret > 0); + if (ret == -2) + return -2; if (lstream->v_size != (uoff_t)-1) { left = lstream->v_size - stream->istream.v_offset; diff --git a/src/lib/istream-rawlog.c b/src/lib/istream-rawlog.c index 7e17d6049e..aa0a75f0f8 100644 --- a/src/lib/istream-rawlog.c +++ b/src/lib/istream-rawlog.c @@ -51,13 +51,13 @@ static ssize_t i_stream_rawlog_read(struct istream_private *stream) if (pos > stream->pos) ret = 0; else do { - if ((ret = i_stream_read_memarea(stream->parent)) == -2) - return -2; - + ret = i_stream_read_memarea(stream->parent); stream->istream.stream_errno = stream->parent->stream_errno; stream->istream.eof = stream->parent->eof; stream->buffer = i_stream_get_data(stream->parent, &pos); } while (pos <= stream->pos && ret > 0); + if (ret == -2) + return -2; if (pos <= stream->pos) ret = ret == 0 ? 0 : -1; diff --git a/src/lib/istream-sized.c b/src/lib/istream-sized.c index eed4e6c640..d5c1e863bd 100644 --- a/src/lib/istream-sized.c +++ b/src/lib/istream-sized.c @@ -50,9 +50,7 @@ i_stream_sized_parent_read(struct istream_private *stream, size_t *pos_r) ssize_t ret; do { - if ((ret = i_stream_read_memarea(stream->parent)) == -2) - break; - + ret = i_stream_read_memarea(stream->parent); stream->istream.stream_errno = stream->parent->stream_errno; stream->istream.eof = stream->parent->eof; stream->buffer = i_stream_get_data(stream->parent, pos_r); diff --git a/src/lib/istream.c b/src/lib/istream.c index e04fb50c6f..161f4a6b2f 100644 --- a/src/lib/istream.c +++ b/src/lib/istream.c @@ -384,11 +384,7 @@ ssize_t i_stream_read_copy_from_parent(struct istream *istream) if (pos > stream->pos) ret = 0; else do { - if ((ret = i_stream_read_memarea(stream->parent)) == -2) { - i_stream_update(stream); - return -2; - } - + ret = i_stream_read_memarea(stream->parent); stream->istream.stream_errno = stream->parent->stream_errno; stream->istream.eof = stream->parent->eof; stream->buffer = i_stream_get_data(stream->parent, &pos); @@ -396,6 +392,10 @@ ssize_t i_stream_read_copy_from_parent(struct istream *istream) backwards and the previous read() didn't get us far enough. */ } while (pos <= stream->pos && ret > 0); + if (ret == -2) { + i_stream_update(stream); + return -2; + } ret = pos > stream->pos ? (ssize_t)(pos - stream->pos) : (ret == 0 ? 0 : -1); diff --git a/src/lib/test-istream-seekable.c b/src/lib/test-istream-seekable.c index fd44f3e105..594e0321a6 100644 --- a/src/lib/test-istream-seekable.c +++ b/src/lib/test-istream-seekable.c @@ -1,7 +1,10 @@ /* Copyright (c) 2009-2018 Dovecot authors, see the included COPYING file */ #include "test-lib.h" +#include "sha2.h" #include "istream-private.h" +#include "istream-sized.h" +#include "istream-hash.h" #include "istream-seekable.h" #include @@ -191,6 +194,28 @@ static void test_istream_seekable_early_end(void) test_end(); } +static void test_istream_seekable_invalid_read(void) +{ + test_begin("istream seekable + other streams causing invalid read"); + struct sha256_ctx hash_ctx; + sha256_init(&hash_ctx); + struct istream *str_input = test_istream_create("123456"); + str_input->seekable = FALSE; + struct istream *seek_inputs[] = { str_input, NULL }; + struct istream *seek_input = i_stream_create_seekable(seek_inputs, 3, fd_callback, NULL); + struct istream *sized_input = i_stream_create_sized(seek_input, 3); + struct istream *input = i_stream_create_hash(sized_input, &hash_method_sha256, &hash_ctx); + test_assert(i_stream_read(input) == 3); + test_assert(i_stream_read(input) == -2); + i_stream_skip(input, 3); + test_assert(i_stream_read(input) == -1); + i_stream_unref(&input); + i_stream_unref(&sized_input); + i_stream_unref(&seek_input); + i_stream_unref(&str_input); + test_end(); +} + void test_istream_seekable(void) { unsigned int i; @@ -208,4 +233,5 @@ void test_istream_seekable(void) test_istream_seekable_eof(); test_istream_seekable_early_end(); + test_istream_seekable_invalid_read(); }