dovecot-2.2: lib-charset: Fixed assert-crash with some iconv() v...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.2/rev/cb4fcdc716e2
changeset: 19127:cb4fcdc716e2
user:      Timo Sirainen <tss at iki.fi>
date:      Tue Sep 08 13:07:59 2015 +0300
description:
lib-charset: Fixed assert-crash with some iconv() versions.
Older glibc iconv() versions seem to skip over invalid characters, at least
with some charsets, while newer versions don't. We were assuming that the
skipping never happened, so if the invalid character was at the end of the
string we could have wrapped size to (size_t)-1 and caused a crash later on.

diffstat:

 src/lib-charset/charset-iconv.c |   4 +++-
 src/lib-charset/test-charset.c  |  23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

diffs (61 lines):

diff -r 793bf4ab439f -r cb4fcdc716e2 src/lib-charset/charset-iconv.c
--- a/src/lib-charset/charset-iconv.c	Sat Apr 25 11:42:06 2015 +0200
+++ b/src/lib-charset/charset-iconv.c	Tue Sep 08 13:07:59 2015 +0300
@@ -111,6 +111,7 @@
 	bool ret;
 
 	for (pos = 0;;) {
+		i_assert(pos <= *src_size);
 		size = *src_size - pos;
 		ret = charset_to_utf8_try(t, src + pos, &size, dest, &result);
 		pos += size;
@@ -124,7 +125,8 @@
 					      strlen(UNICODE_REPLACEMENT_CHAR_UTF8));
 				prev_invalid_pos = dest->used;
 			}
-			pos++;
+			if (pos < *src_size)
+				pos++;
 		}
 	}
 
diff -r 793bf4ab439f -r cb4fcdc716e2 src/lib-charset/test-charset.c
--- a/src/lib-charset/test-charset.c	Sat Apr 25 11:42:06 2015 +0200
+++ b/src/lib-charset/test-charset.c	Tue Sep 08 13:07:59 2015 +0300
@@ -85,6 +85,28 @@
 	test_charset_utf8_common("UTF-8//IGNORE");
 	test_end();
 }
+static void test_charset_iconv_crashes(void)
+{
+	struct {
+		const char *charset;
+		const char *input;
+	} tests[] = {
+		{ "CP932", "\203\334" }
+	};
+	string_t *str = t_str_new(128);
+	enum charset_result result;
+	unsigned int i;
+
+	test_begin("charset iconv crashes");
+	for (i = 0; i < N_ELEMENTS(tests); i++) {
+		str_truncate(str, 0);
+		/* we don't care about checking the result. we only want to
+		   verify that there's no crash. */
+		(void)charset_to_utf8_str(tests[i].charset, NULL,
+					  tests[i].input, str, &result);
+	}
+	test_end();
+}
 #endif
 
 int main(void)
@@ -94,6 +116,7 @@
 		test_charset_utf8,
 #ifdef HAVE_ICONV
 		test_charset_iconv,
+		test_charset_iconv_crashes,
 #endif
 		NULL
 	};