dovecot-2.2: lib-mail: Detect errors in quoted-printable input.

dovecot at dovecot.org dovecot at dovecot.org
Tue Dec 4 14:10:41 EET 2012


details:   http://hg.dovecot.org/dovecot-2.2/rev/0be51d94b0d9
changeset: 15453:0be51d94b0d9
user:      Timo Sirainen <tss at iki.fi>
date:      Tue Dec 04 14:10:34 2012 +0200
description:
lib-mail: Detect errors in quoted-printable input.

diffstat:

 src/lib-mail/istream-qp-decoder.c      |  27 +++++++-------
 src/lib-mail/message-decoder.c         |  10 ++--
 src/lib-mail/message-header-decode.c   |   8 ++-
 src/lib-mail/quoted-printable.c        |  36 +++++++++++++++---
 src/lib-mail/quoted-printable.h        |  15 +++++--
 src/lib-mail/test-istream-qp-decoder.c |   3 +-
 src/lib-mail/test-message-decoder.c    |   5 +-
 src/lib-mail/test-quoted-printable.c   |  62 ++++++++++++++++++++++++++-------
 8 files changed, 115 insertions(+), 51 deletions(-)

diffs (truncated from 361 to 300 lines):

diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/istream-qp-decoder.c
--- a/src/lib-mail/istream-qp-decoder.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/istream-qp-decoder.c	Tue Dec 04 14:10:34 2012 +0200
@@ -35,12 +35,13 @@
 }
 
 static int
-i_stream_qp_try_decode_block(struct qp_decoder_istream *bstream)
+i_stream_qp_try_decode_block(struct qp_decoder_istream *bstream, bool eof)
 {
 	struct istream_private *stream = &bstream->istream;
 	const unsigned char *data;
 	size_t size, avail, buffer_avail, pos;
 	buffer_t buf;
+	int ret;
 
 	data = i_stream_get_data(stream->parent, &size);
 	if (size == 0)
@@ -59,7 +60,12 @@
 
 	buffer_create_from_data(&buf, stream->w_buffer + stream->pos,
 				buffer_avail);
-	quoted_printable_decode(data, size, &pos, &buf);
+	ret = !eof ? quoted_printable_decode(data, size, &pos, &buf) :
+		quoted_printable_decode_final(data, size, &pos, &buf);
+	if (ret < 0) {
+		stream->istream.stream_errno = EINVAL;
+		return -1;
+	}
 
 	stream->pos += buf.used;
 	i_stream_skip(stream->parent, pos);
@@ -70,8 +76,6 @@
 {
 	struct qp_decoder_istream *bstream =
 		(struct qp_decoder_istream *)stream;
-	const unsigned char *data;
-	size_t size;
 	size_t pre_count, post_count;
 	int ret;
 	size_t prev_size = 0;
@@ -82,26 +86,21 @@
 			if (ret != -1 || stream->istream.stream_errno != 0)
 				return 0;
 
-			data = i_stream_get_data(stream->parent, &size);
-			if (size == 0)
-				return -1;
-
-			if (size == 1 && data[0] == '=') {
-				/* ends with "=". normally this would be
-				   followed by LF, but it's not really an
-				   error even without. */
-				i_stream_skip(stream->parent, 1);
+			ret = i_stream_qp_try_decode_block(bstream, TRUE);
+			if (ret == 0) {
+				/* ended with =[whitespace] but without LF */
 				stream->istream.eof = TRUE;
 				return -1;
 			}
 			/* qp input with a partial block */
+			i_assert(ret < 0);
 			stream->istream.stream_errno = EINVAL;
 			return -1;
 		}
 
 		/* encode as many blocks as fits into destination buffer */
 		pre_count = stream->pos - stream->skip;
-		while ((ret = i_stream_qp_try_decode_block(bstream)) > 0) ;
+		while ((ret = i_stream_qp_try_decode_block(bstream, FALSE)) > 0) ;
 		post_count = stream->pos - stream->skip;
 	} while (ret == 0 && pre_count == post_count);
 
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/message-decoder.c
--- a/src/lib-mail/message-decoder.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/message-decoder.c	Tue Dec 04 14:10:34 2012 +0200
@@ -281,12 +281,12 @@
 	case MESSAGE_CTE_QP:
 		buffer_set_used_size(ctx->buf, 0);
 		if (ctx->encoding_buf->used != 0) {
-			quoted_printable_decode(ctx->encoding_buf->data,
-						ctx->encoding_buf->used,
-						&pos, ctx->buf);
+			(void)quoted_printable_decode(ctx->encoding_buf->data,
+						      ctx->encoding_buf->used,
+						      &pos, ctx->buf);
 		} else {
-			quoted_printable_decode(input->data, input->size,
-						&pos, ctx->buf);
+			(void)quoted_printable_decode(input->data, input->size,
+						      &pos, ctx->buf);
 		}
 		data = ctx->buf->data;
 		size = ctx->buf->used;
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/message-header-decode.c
--- a/src/lib-mail/message-header-decode.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/message-header-decode.c	Tue Dec 04 14:10:34 2012 +0200
@@ -36,9 +36,11 @@
 	switch (data[start_pos[0]+1]) {
 	case 'q':
 	case 'Q':
-		quoted_printable_q_decode(data + start_pos[1] + 1,
-					  start_pos[2] - start_pos[1] - 1,
-					  decodebuf);
+		if (quoted_printable_q_decode(data + start_pos[1] + 1,
+					      start_pos[2] - start_pos[1] - 1,
+					      decodebuf) < 0) {
+			/* we skipped over some invalid data */
+		}
 		break;
 	case 'b':
 	case 'B':
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/quoted-printable.c
--- a/src/lib-mail/quoted-printable.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/quoted-printable.c	Tue Dec 04 14:10:34 2012 +0200
@@ -27,11 +27,13 @@
 	return -1;
 }
 
-void quoted_printable_decode(const unsigned char *src, size_t src_size,
-			     size_t *src_pos_r, buffer_t *dest)
+static int
+quoted_printable_decode_full(const unsigned char *src, size_t src_size,
+			     size_t *src_pos_r, buffer_t *dest, bool eof)
 {
 	char hexbuf[3];
 	size_t src_pos, pos, next;
+	bool errors = FALSE;
 	int ret;
 
 	hexbuf[2] = '\0';
@@ -64,11 +66,15 @@
 			continue;
 		}
 		if (ret < 0) {
-			/* unknown yet if this is end of line */
+			/* '=' was followed only by whitespace */
 			break;
 		}
-		if (src_pos+2 >= src_size)
+		if (src_pos+2 >= src_size) {
+			/* '=' was followed by non-whitespace */
+			if (eof)
+				errors = TRUE;
 			break;
+		}
 
 		/* =<hex> */
 		hexbuf[0] = src[src_pos+1];
@@ -79,6 +85,7 @@
 			next = src_pos + 1;
 		} else {
 			/* non-hex data, show as-is */
+			errors = TRUE;
 			next = src_pos;
 		}
 	}
@@ -91,15 +98,28 @@
 		buffer_append(dest, src + next, src_pos - next);
 		next = src_pos;
 	}
-
 	*src_pos_r = next;
+	return errors ? -1 : 0;
 }
 
-void quoted_printable_q_decode(const unsigned char *src, size_t src_size,
-			       buffer_t *dest)
+int quoted_printable_decode(const unsigned char *src, size_t src_size,
+			    size_t *src_pos_r, buffer_t *dest)
+{
+	return quoted_printable_decode_full(src, src_size, src_pos_r, dest, FALSE);
+}
+
+int quoted_printable_decode_final(const unsigned char *src, size_t src_size,
+				  size_t *src_pos_r, buffer_t *dest)
+{
+	return quoted_printable_decode_full(src, src_size, src_pos_r, dest, TRUE);
+}
+
+int quoted_printable_q_decode(const unsigned char *src, size_t src_size,
+			      buffer_t *dest)
 {
 	char hexbuf[3];
 	size_t src_pos, next;
+	bool errors = FALSE;
 
 	hexbuf[2] = '\0';
 
@@ -129,8 +149,10 @@
 			next = src_pos+1;
 		} else {
 			/* non-hex data, show as-is */
+			errors = TRUE;
 			next = src_pos;
 		}
 	}
 	buffer_append(dest, src + next, src_size - next);
+	return errors ? -1 : 0;
 }
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/quoted-printable.h
--- a/src/lib-mail/quoted-printable.h	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/quoted-printable.h	Tue Dec 04 14:10:34 2012 +0200
@@ -2,14 +2,19 @@
 #define QUOTED_PRINTABLE_H
 
 /* Translates quoted printable data into binary. dest must be at least the
-   size of src, and may be same as src. Decoding errors are ignored.
+   size of src, and may be same as src. Returns 0 if input was valid, -1 if
+   there were some decoding errors (which were skipped over).
 
    This function may be called multiple times for parsing the same stream.
    src_pos is updated to first non-translated character in src. */
-void quoted_printable_decode(const unsigned char *src, size_t src_size,
-			     size_t *src_pos_r, buffer_t *dest);
+int quoted_printable_decode(const unsigned char *src, size_t src_size,
+			    size_t *src_pos_r, buffer_t *dest);
+/* Like quoted_printable_decode(), but handle src as the final block.
+   This allows src to end without LF. */
+int quoted_printable_decode_final(const unsigned char *src, size_t src_size,
+				  size_t *src_pos_r, buffer_t *dest);
 /* Decode MIME "Q" encoding. */
-void quoted_printable_q_decode(const unsigned char *src, size_t src_size,
-			       buffer_t *dest);
+int quoted_printable_q_decode(const unsigned char *src, size_t src_size,
+			      buffer_t *dest);
 
 #endif
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/test-istream-qp-decoder.c
--- a/src/lib-mail/test-istream-qp-decoder.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/test-istream-qp-decoder.c	Tue Dec 04 14:10:34 2012 +0200
@@ -10,7 +10,8 @@
 	const char *output;
 } tests[] = {
 	{ "p=C3=A4=C3=A4t=C3=B6s", "päätös" },
-	{ "p=c3=a4=c3=a4t=c3=b6s", "päätös" },
+	{ "p=c3=a4=c3=a4t=c3=b6s=  ", "päätös" },
+	{ "p=c3=a4=c3=a4t=c3=b6s=  \n", "päätös" },
 	{ "p=c3=a4= \t \n=c3=\r\n=a4t=  \r\n=c3=b6s", "päätös" },
 };
 
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/test-message-decoder.c
--- a/src/lib-mail/test-message-decoder.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/test-message-decoder.c	Tue Dec 04 14:10:34 2012 +0200
@@ -16,13 +16,14 @@
 	buffer_append(dest, data, size);
 }
 
-void quoted_printable_decode(const unsigned char *src, size_t src_size,
-			     size_t *src_pos_r, buffer_t *dest)
+int quoted_printable_decode(const unsigned char *src, size_t src_size,
+			    size_t *src_pos_r, buffer_t *dest)
 {
 	while (src_size > 0 && src[src_size-1] == ' ')
 		src_size--;
 	buffer_append(dest, src, src_size);
 	*src_pos_r = src_size;
+	return 0;
 }
 
 int charset_to_utf8_begin(const char *charset ATTR_UNUSED,
diff -r 266e24f1c78c -r 0be51d94b0d9 src/lib-mail/test-quoted-printable.c
--- a/src/lib-mail/test-quoted-printable.c	Tue Dec 04 13:04:59 2012 +0200
+++ b/src/lib-mail/test-quoted-printable.c	Tue Dec 04 14:10:34 2012 +0200
@@ -10,34 +10,38 @@
 	const char *input;
 	const char *output;
 	int end_skip;
+	int ret;
 };
 
 static void test_quoted_printable_decode(void)
 {
 	static struct test_quoted_printable_decode_data data[] = {
-		{ "foo  \r\nbar=", "foo\r\nbar", 1 },
-		{ "foo\t=\nbar", "foo\tbar", 0 },
-		{ "foo = \n=01", "foo \001", 0 },
-		{ "foo =\t\r\nbar", "foo bar", 0 },
-		{ "foo =\r\n=01", "foo \001", 0 },
-		{ "foo  \nbar=", "foo\r\nbar", 1 },
-		{ "=0A=0D  ", "\n\r", 2 },
-		{ "foo_bar", "foo_bar", 0 },
-		{ "foo=", "foo", 1 },
-		{ "foo=A", "foo", 2 },
-		{ "foo=Ax", "foo=Ax", 0 },
-		{ "foo=Ax=xy", "foo=Ax=xy", 0 }
+		{ "foo  \r\nbar=", "foo\r\nbar", 1, 0 },
+		{ "foo\t=\nbar", "foo\tbar", 0, 0 },
+		{ "foo = \n=01", "foo \001", 0, 0 },
+		{ "foo =\t\r\nbar", "foo bar", 0, 0 },
+		{ "foo =\r\n=01", "foo \001", 0, 0 },
+		{ "foo  \nbar=", "foo\r\nbar", 1, 0 },
+		{ "=0A=0D  ", "\n\r", 2, 0 },
+		{ "foo_bar", "foo_bar", 0, 0 },
+		{ "foo=", "foo", 1, 0 },
+		{ "foo=  ", "foo", 3, 0 },
+		{ "foo=A", "foo", 2, 0 },
+		{ "foo=Ax", "foo=Ax", 0, -1 },
+		{ "foo=Ax=xy", "foo=Ax=xy", 0, -1 }
 	};


More information about the dovecot-cvs mailing list