CVE-2015-5330: Fix handling of unicode near string endings

Until now next_codepoint_ext() and next_codepoint_handle_ext() were
using strnlen(str, 5) to determine how much string they should try to
decode. This ended up looking past the end of the string when it was not
null terminated and the final character looked like a multi-byte encoding.
The fix is to let the caller say how long the string can be.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

4 files changed, 25 insertions, 14 deletions

diff --git a/lib/util/charset/charset.h b/lib/util/charset/charset.h
index e4297e4f3ce..060f1cf56f7 100644
--- a/lib/util/charset/charset.h
+++ b/lib/util/charset/charset.h
@@ -171,15 +171,16 @@ smb_iconv_t get_conv_handle(struct smb_iconv_handle *ic,
 			    charset_t from, charset_t to);
 const char *charset_name(struct smb_iconv_handle *ic, charset_t ch);
 
-codepoint_t next_codepoint_ext(const char *str, charset_t src_charset,
-			       size_t *size);
+codepoint_t next_codepoint_ext(const char *str, size_t len,
+			       charset_t src_charset, size_t *size);
 codepoint_t next_codepoint(const char *str, size_t *size);
 ssize_t push_codepoint(char *str, codepoint_t c);
 
 /* codepoints */
 codepoint_t next_codepoint_handle_ext(struct smb_iconv_handle *ic,
-			    const char *str, charset_t src_charset,
-			    size_t *size);
+				      const char *str, size_t len,
+				      charset_t src_charset,
+				      size_t *size);
 codepoint_t next_codepoint_handle(struct smb_iconv_handle *ic,
 			    const char *str, size_t *size);
 ssize_t push_codepoint_handle(struct smb_iconv_handle *ic,
diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
index 0984164d483..542eeae73a5 100644
--- a/lib/util/charset/codepoints.c
+++ b/lib/util/charset/codepoints.c
@@ -319,7 +319,8 @@ smb_iconv_t get_conv_handle(struct smb_iconv_handle *ic,
  */
 _PUBLIC_ codepoint_t next_codepoint_handle_ext(
 			struct smb_iconv_handle *ic,
-			const char *str, charset_t src_charset,
+			const char *str, size_t len,
+			charset_t src_charset,
 			size_t *bytes_consumed)
 {
 	/* it cannot occupy more than 4 bytes in UTF16 format */
@@ -339,7 +340,7 @@ _PUBLIC_ codepoint_t next_codepoint_handle_ext(
 	 * we assume that no multi-byte character can take more than 5 bytes.
 	 * This is OK as we only support codepoints up to 1M (U+100000)
 	 */
-	ilen_orig = strnlen(str, 5);
+	ilen_orig = MIN(len, 5);
 	ilen = ilen_orig;
 
 	descriptor = get_conv_handle(ic, src_charset, CH_UTF16);
@@ -395,9 +396,16 @@ _PUBLIC_ codepoint_t next_codepoint_handle_ext(
   return INVALID_CODEPOINT if the next character cannot be converted
 */
 _PUBLIC_ codepoint_t next_codepoint_handle(struct smb_iconv_handle *ic,
-				    const char *str, size_t *size)
+					   const char *str, size_t *size)
 {
-	return next_codepoint_handle_ext(ic, str, CH_UNIX, size);
+	/*
+	 * We assume that no multi-byte character can take more than 5 bytes
+	 * thus avoiding walking all the way down a long string. This is OK as
+	 * Unicode codepoints only go up to (U+10ffff), which can always be
+	 * encoded in 4 bytes or less.
+	 */
+	return next_codepoint_handle_ext(ic, str, strnlen(str, 5), CH_UNIX,
+					 size);
 }
 
 /*
@@ -459,11 +467,11 @@ _PUBLIC_ ssize_t push_codepoint_handle(struct smb_iconv_handle *ic,
 	return 5 - olen;
 }
 
-_PUBLIC_ codepoint_t next_codepoint_ext(const char *str, charset_t src_charset,
-					size_t *size)
+_PUBLIC_ codepoint_t next_codepoint_ext(const char *str, size_t len,
+					charset_t src_charset, size_t *size)
 {
-	return next_codepoint_handle_ext(get_iconv_handle(), str,
-					      src_charset, size);
+	return next_codepoint_handle_ext(get_iconv_handle(), str, len,
+					 src_charset, size);
 }
 
 _PUBLIC_ codepoint_t next_codepoint(const char *str, size_t *size)
diff --git a/lib/util/charset/util_str.c b/lib/util/charset/util_str.c
index d2e6cbbc620..2653bfc2d81 100644
--- a/lib/util/charset/util_str.c
+++ b/lib/util/charset/util_str.c
@@ -210,7 +210,8 @@ _PUBLIC_ size_t strlen_m_ext_handle(struct smb_iconv_handle *ic,
 
 	while (*s) {
 		size_t c_size;
-		codepoint_t c = next_codepoint_handle_ext(ic, s, src_charset, &c_size);
+		codepoint_t c = next_codepoint_handle_ext(ic, s, strnlen(s, 5),
+							  src_charset, &c_size);
 		s += c_size;
 
 		switch (dst_charset) {
diff --git a/lib/util/charset/util_unistr.c b/lib/util/charset/util_unistr.c
index e4ae65053c7..f2992695f65 100644
--- a/lib/util/charset/util_unistr.c
+++ b/lib/util/charset/util_unistr.c
@@ -112,7 +112,8 @@ _PUBLIC_ char *strupper_talloc_n_handle(struct smb_iconv_handle *iconv_handle,
 
 	while (n-- && *src) {
 		size_t c_size;
-		codepoint_t c = next_codepoint_handle(iconv_handle, src, &c_size);
+		codepoint_t c = next_codepoint_handle_ext(iconv_handle, src, n,
+							  CH_UNIX, &c_size);
 		src += c_size;
 
 		c = toupper_m(c);